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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Let me know if you are OK with this.

Thanks, Roger.