Re: [PATCH v4 1/4] xen/io: provide helpers for multi size MMIO accesses

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.04.2025 12:46, Roger Pau Monné wrote:
> On Tue, Apr 29, 2025 at 12:23:05PM +0200, Jan Beulich wrote:
>> On 29.04.2025 12:12, Roger Pau Monne wrote:
>>> Several handlers have the same necessity of reading or writing from or to
>>> an MMIO region using 1, 2, 4 or 8 bytes accesses.  So far this has been
>>> open-coded in the function itself.  Instead provide a new set of handlers
>>> that encapsulate the accesses.
>>>
>>> Since the added helpers are not architecture specific, introduce a new
>>> generic io.h header.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> preferably with ...
>>
>>> @@ -5185,6 +5167,7 @@ int cf_check mmio_ro_emulated_write(
>>>      struct x86_emulate_ctxt *ctxt)
>>>  {
>>>      struct mmio_ro_emulate_ctxt *mmio_ro_ctxt = ctxt->data;
>>> +    unsigned long data = 0;
>>
>>
>> ... this moved ...
>>
>>> @@ -5195,9 +5178,12 @@ int cf_check mmio_ro_emulated_write(
>>>          return X86EMUL_UNHANDLEABLE;
>>>      }
>>>  
>>> -    if ( bytes <= 8 )
>>> +    if ( bytes <= sizeof(data) )
>>> +    {
>>> +        memcpy(&data, p_data, bytes);
>>>          subpage_mmio_write_emulate(mmio_ro_ctxt->mfn, PAGE_OFFSET(offset),
>>> -                                   p_data, bytes);
>>> +                                   data, bytes);
>>> +    }
>>
>> ... into this more narrow scope.
> 
> Hm, but if I move it I can no longer use sizeof(data) in the condition
> check, that's why data is defined in the outside.

Oh, I didn't pay attention to that. Yes - please keep as you have it.

Jan