Re: [PATCH v3] tcg: Fix execution on Apple Silicon

[Richard Henderson]

On 1/29/21 10:50 AM, Roman Bolshakov wrote:
> On Fri, Jan 29, 2021 at 10:18:58AM -1000, Richard Henderson wrote:
>> On 1/21/21 8:34 AM, Richard Henderson wrote:
>>> On 1/12/21 5:28 PM, Roman Bolshakov wrote:
>>>> @@ -1083,6 +1083,12 @@ static bool alloc_code_gen_buffer_anon(size_t size, 
>>>> int prot,
>>>>  {
>>>>      void *buf;
>>>>  
>>>> +#if defined(MAC_OS_VERSION_11_0) && \
>>>> +    MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0
>>>> +    if (__builtin_available(macOS 11.0, *)) {
>>>> +        flags |= MAP_JIT;
>>>> +    }
>>>> +#endif
>>>
>>> This hunk should be in alloc_code_gen_buffer, where we do the other flags
>>> manipulation.
>>>
>>> I'll drop this hunk and apply the rest, which is exclusively related to
>>> toggling the jit bit.
>>
>> Ping on this?
>>
> Hi Richard,
> 
>> I would imagine that the patch would look something like
>>
>> --- a/accel/tcg/translate-all.c
>> +++ b/accel/tcg/translate-all.c
>> @@ -1296,6 +1296,11 @@ static bool alloc_code_gen_buffer
>>  #ifdef CONFIG_TCG_INTERPRETER
>>      /* The tcg interpreter does not need execute permission. */
>>      prot = PROT_READ | PROT_WRITE;
>> +#elif defined(MAC_OS_VERSION_11_0) && \
>> +    MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0
>> +    if (__builtin_available(macOS 11.0, *)) {
>> +        flags |= MAP_JIT;
>> +    }
>>  #elif defined(CONFIG_DARWIN)
>>      /* Applicable to both iOS and macOS (Apple Silicon). */
>>      if (!splitwx) {
>>
>> But I don't know how CONFIG_DARWIN, iOS, and MAC_OS_VERSION interact, and I'm
>> not able to even compile-test the patch.
>> Certainly the final comment there looks suspicious, given the preceding 
>> MAC_OS
>> stanza...
>>
> 
> I thought you already added MAP_JIT in 6f70ddee19e. It's getting enabled
> on my M1 laptop. Was it intended or not?
> 
>     /* Applicable to both iOS and macOS (Apple Silicon). */
>     if (!splitwx) {
>         flags |= MAP_JIT;
>     }
> 
> TCG from master branch of QEMU works fine on M1. I'm not sure why do we
> need to duplicate it.

I thought there was something about abi/api build issues.  If there's nothing
that needs doing, great!


r~