Re: [Xen-devel] [PATCH] Avoid cpu_physical_memory_rw() with a constant is_write argument

[Peter Maydell <peter.maydell@xxxxxxxxxx>]

On Tue, 18 Feb 2020 at 17:57, Stefan Weil <sw@xxxxxxxxxxx> wrote:
>
> Am 18.02.20 um 14:20 schrieb Philippe Mathieu-Daudé:
>
> > This commit was produced with the included Coccinelle script
> > scripts/coccinelle/as-rw-const.patch.
> >
> > Inspired-by: Peter Maydell <peter.maydell@xxxxxxxxxx>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> > ---
> > Based-on: <20200218112457.22712-1-peter.maydell@xxxxxxxxxx>
> [...]
> > diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> > index a8b6e5aeb8..f5971ccc74 100644
> > --- a/target/i386/hax-all.c
> > +++ b/target/i386/hax-all.c
> > @@ -376,8 +376,8 @@ static int hax_handle_fastmmio(CPUArchState *env, 
> > struct hax_fastmmio *hft)
> >           *  hft->direction == 2: gpa ==> gpa2
> >           */
> >          uint64_t value;
> > -        cpu_physical_memory_rw(hft->gpa, (uint8_t *) &value, hft->size, 0);
> > -        cpu_physical_memory_rw(hft->gpa2, (uint8_t *) &value, hft->size, 
> > 1);
> > +        cpu_physical_memory_read(hft->gpa, (uint8_t *)&value, hft->size);
> > +        cpu_physical_memory_write(hft->gpa2, (uint8_t *)&value, hft->size);
>
>
> Maybe those type casts could be removed, too. They are no longer needed
> after your modification.

I think that we should fix the inconsistency where these functions
all take "uint8_t* buf":

 - address_space_rw()
 - address_space_read()
 - address_space_write()
 - address_space_write_rom()
 - cpu_physical_memory_rw()
 - cpu_memory_rw_debug()

but these take void*:
 - cpu_physical_memory_read()
 - cpu_physical_memory_write()
 - address_space_write_cached()
 - address_space_read_cached_slow()
 - address_space_write_cached_slow()
 - pci_dma_read()
 - pci_dma_write()
 - pci_dma_rw()
 - dma_memory_read()
 - dma_memory_write()
 - dma_memory_rw()
 - dma_memory_rw_relaxed()

Depending on which way we go we would either want to remove these
casts, or not.

I guess that we have more cases of 'void*', and that would
certainly be the easier way to convert (otherwise we probably
need to add a bunch of new casts to uint8_t* in various callsites),
but I don't have a strong opinion. Paolo ?

thanks
-- PMM

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel