Re: [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
On Fri, Feb 09, 2018 at 05:05:23PM -0300, Philippe Mathieu-Daudé wrote: > Hi Mark, > > On 02/09/2018 03:51 PM, Mark Cave-Ayland wrote: > > Signed-off-by: Mark Cave-Ayland> > --- > > hw/misc/macio/cuda.c | 40 > > 1 file changed, 8 insertions(+), 32 deletions(-) > > > > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c > > index 008d8bd4d5..6631017ca2 100644 > > --- a/hw/misc/macio/cuda.c > > +++ b/hw/misc/macio/cuda.c > > @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s) > > timer_mod(s->sr_delay_timer, expire); > > } > > > > -static uint32_t cuda_readb(void *opaque, hwaddr addr) > > +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size) > > { > > CUDAState *s = opaque; > > uint32_t val; > > @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr) > > return val; > > } > > > > -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val) > > +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned > > size) > > { > > CUDAState *s = opaque; > > > > @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState > > *s, > > } > > } > > > > -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value) > > -{ > > -} > > - > > -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value) > > -{ > > -} > > - > > -static uint32_t cuda_readw (void *opaque, hwaddr addr) > > -{ > > -return 0; > > -} > > - > > -static uint32_t cuda_readl (void *opaque, hwaddr addr) > > -{ > > -return 0; > > -} > > - > > static const MemoryRegionOps cuda_ops = { > > -.old_mmio = { > > -.write = { > > -cuda_writeb, > > -cuda_writew, > > -cuda_writel, > > -}, > > -.read = { > > -cuda_readb, > > -cuda_readw, > > -cuda_readl, > > -}, > > +.read = cuda_read, > > +.write = cuda_write, > > +.endianness = DEVICE_BIG_ENDIAN, > > +.valid = { > > This change the device behavior, previously 16/32bit access were > ignored, now they are illegal. This change looks correct to me. As you note >1 byte accesses were previously ignored - not broken or handled in any other way. That means there's no possibility using >1byte accesses could ever accomplish, so it reads to me as being invalid. I'd say the fact that it previously didn't generate some sort of error is the bug. > Using ".impl" instead also change the behavior, since 16/32bits access > would be processed (as 2x or 4x 8bit access). > > A comment "this change is safe because ..." would reassure me ;) > > > +.min_access_size = 1, > > +.max_access_size = 1, > > }, > > -.endianness = DEVICE_NATIVE_ENDIAN, > > }; > > > > static bool cuda_timer_exist(void *opaque, int version_id) > > > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
On Fri, Feb 09, 2018 at 06:51:31PM +, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-AylandApplied, thanks. > --- > hw/misc/macio/cuda.c | 40 > 1 file changed, 8 insertions(+), 32 deletions(-) > > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c > index 008d8bd4d5..6631017ca2 100644 > --- a/hw/misc/macio/cuda.c > +++ b/hw/misc/macio/cuda.c > @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s) > timer_mod(s->sr_delay_timer, expire); > } > > -static uint32_t cuda_readb(void *opaque, hwaddr addr) > +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size) > { > CUDAState *s = opaque; > uint32_t val; > @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr) > return val; > } > > -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val) > +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned > size) > { > CUDAState *s = opaque; > > @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s, > } > } > > -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value) > -{ > -} > - > -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value) > -{ > -} > - > -static uint32_t cuda_readw (void *opaque, hwaddr addr) > -{ > -return 0; > -} > - > -static uint32_t cuda_readl (void *opaque, hwaddr addr) > -{ > -return 0; > -} > - > static const MemoryRegionOps cuda_ops = { > -.old_mmio = { > -.write = { > -cuda_writeb, > -cuda_writew, > -cuda_writel, > -}, > -.read = { > -cuda_readb, > -cuda_readw, > -cuda_readl, > -}, > +.read = cuda_read, > +.write = cuda_write, > +.endianness = DEVICE_BIG_ENDIAN, > +.valid = { > +.min_access_size = 1, > +.max_access_size = 1, > }, > -.endianness = DEVICE_NATIVE_ENDIAN, > }; > > static bool cuda_timer_exist(void *opaque, int version_id) -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
Hi Mark, On 02/09/2018 03:51 PM, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-Ayland> --- > hw/misc/macio/cuda.c | 40 > 1 file changed, 8 insertions(+), 32 deletions(-) > > diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c > index 008d8bd4d5..6631017ca2 100644 > --- a/hw/misc/macio/cuda.c > +++ b/hw/misc/macio/cuda.c > @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s) > timer_mod(s->sr_delay_timer, expire); > } > > -static uint32_t cuda_readb(void *opaque, hwaddr addr) > +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size) > { > CUDAState *s = opaque; > uint32_t val; > @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr) > return val; > } > > -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val) > +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned > size) > { > CUDAState *s = opaque; > > @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s, > } > } > > -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value) > -{ > -} > - > -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value) > -{ > -} > - > -static uint32_t cuda_readw (void *opaque, hwaddr addr) > -{ > -return 0; > -} > - > -static uint32_t cuda_readl (void *opaque, hwaddr addr) > -{ > -return 0; > -} > - > static const MemoryRegionOps cuda_ops = { > -.old_mmio = { > -.write = { > -cuda_writeb, > -cuda_writew, > -cuda_writel, > -}, > -.read = { > -cuda_readb, > -cuda_readw, > -cuda_readl, > -}, > +.read = cuda_read, > +.write = cuda_write, > +.endianness = DEVICE_BIG_ENDIAN, > +.valid = { This change the device behavior, previously 16/32bit access were ignored, now they are illegal. Using ".impl" instead also change the behavior, since 16/32bits access would be processed (as 2x or 4x 8bit access). A comment "this change is safe because ..." would reassure me ;) > +.min_access_size = 1, > +.max_access_size = 1, > }, > -.endianness = DEVICE_NATIVE_ENDIAN, > }; > > static bool cuda_timer_exist(void *opaque, int version_id) >
[Qemu-devel] [PATCHv2 01/12] cuda: do not use old_mmio accesses
Signed-off-by: Mark Cave-Ayland--- hw/misc/macio/cuda.c | 40 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index 008d8bd4d5..6631017ca2 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -275,7 +275,7 @@ static void cuda_delay_set_sr_int(CUDAState *s) timer_mod(s->sr_delay_timer, expire); } -static uint32_t cuda_readb(void *opaque, hwaddr addr) +static uint64_t cuda_read(void *opaque, hwaddr addr, unsigned size) { CUDAState *s = opaque; uint32_t val; @@ -350,7 +350,7 @@ static uint32_t cuda_readb(void *opaque, hwaddr addr) return val; } -static void cuda_writeb(void *opaque, hwaddr addr, uint32_t val) +static void cuda_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { CUDAState *s = opaque; @@ -780,38 +780,14 @@ static void cuda_receive_packet_from_host(CUDAState *s, } } -static void cuda_writew (void *opaque, hwaddr addr, uint32_t value) -{ -} - -static void cuda_writel (void *opaque, hwaddr addr, uint32_t value) -{ -} - -static uint32_t cuda_readw (void *opaque, hwaddr addr) -{ -return 0; -} - -static uint32_t cuda_readl (void *opaque, hwaddr addr) -{ -return 0; -} - static const MemoryRegionOps cuda_ops = { -.old_mmio = { -.write = { -cuda_writeb, -cuda_writew, -cuda_writel, -}, -.read = { -cuda_readb, -cuda_readw, -cuda_readl, -}, +.read = cuda_read, +.write = cuda_write, +.endianness = DEVICE_BIG_ENDIAN, +.valid = { +.min_access_size = 1, +.max_access_size = 1, }, -.endianness = DEVICE_NATIVE_ENDIAN, }; static bool cuda_timer_exist(void *opaque, int version_id) -- 2.11.0