Re: [Xen-devel] [PATCH 3/6] x86/shadow: Use ERR_PTR infrastructure for sh_emulate_map_dest()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/06/2017 09:14, Jan Beulich wrote:
>>>> On 21.06.17 at 17:12, <andrew.cooper3@xxxxxxxxxx> wrote:
>> sh_emulate_map_dest() predates the introduction of the generic ERR_PTR()
>> infrasturcture, but take the opportunity to avoid opencoding it.
>>
>> The chosen error constants require need to be negative to work with IS_ERR(),
>> but no other changes.
> Drop one of "require" or "need"?
>
>> --- a/xen/arch/x86/mm/shadow/private.h
>> +++ b/xen/arch/x86/mm/shadow/private.h
>> @@ -395,10 +395,9 @@ void shadow_unhook_mappings(struct domain *d, mfn_t 
>> smfn, int user_only);
>>  
>>  /* Returns a mapped pointer to write to, or one of the following error
>>   * indicators. */
>> -#define MAPPING_UNHANDLEABLE ((void *)(unsigned long)X86EMUL_UNHANDLEABLE)
>> -#define MAPPING_EXCEPTION    ((void *)(unsigned long)X86EMUL_EXCEPTION)
>> -#define MAPPING_SILENT_FAIL  ((void *)(unsigned long)X86EMUL_OKAY)
>> -#define sh_emulate_map_dest_failed(rc) ((unsigned long)(rc) <= 3)
>> +#define MAPPING_UNHANDLEABLE ERR_PTR(~(long)X86EMUL_UNHANDLEABLE)
>> +#define MAPPING_EXCEPTION    ERR_PTR(~(long)X86EMUL_EXCEPTION)
>> +#define MAPPING_SILENT_FAIL  ERR_PTR(~(long)X86EMUL_OKAY)
> While this doesn't change with your patch, this last definition has a
> string dependency on X86EMUL_OKAY to be zero, yet there's no
> respective BUILD_BUG_ON() anywhere. Would you mind adding
> one at once? In any event
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

There is no longer any dependence on X86EMUL_OKAY being 0 (so far as I
can see).  All this logic would work fine if OKAY had the value 10 for
example.

The only requirement (now) is that the X86EMUL constants are all within
MAX_ERRNO so they count as errors as far as the ERR_PTR logic is
concerned, and I suppose those should gain BUILD_BUG_ON()s

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel