[Qemu-devel] [PATCH 1/3] hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
We now support direct execution from MMIO regions in the core memory subsystem. This means that we don't need to have device-specific support for it, and we can remove the request_ptr handling from the Xilinx SPIPS device. (It was broken anyway due to race conditions, and disabled by default.) This device is the only in-tree user of this API. Signed-off-by: Peter Maydell --- hw/ssi/xilinx_spips.c | 46 --- 1 file changed, 46 deletions(-) diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index c052bfc4b3c..16f88f74029 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -1031,14 +1031,6 @@ static const MemoryRegionOps spips_ops = { static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q) { -XilinxSPIPS *s = &q->parent_obj; - -if ((q->mmio_execution_enabled) && (q->lqspi_cached_addr != ~0ULL)) { -/* Invalidate the current mapped mmio */ -memory_region_invalidate_mmio_ptr(&s->mmlqspi, q->lqspi_cached_addr, - LQSPI_CACHE_SIZE); -} - q->lqspi_cached_addr = ~0ULL; } @@ -1207,23 +1199,6 @@ static void lqspi_load_cache(void *opaque, hwaddr addr) } } -static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned *size, -unsigned *offset) -{ -XilinxQSPIPS *q = opaque; -hwaddr offset_within_the_region; - -if (!q->mmio_execution_enabled) { -return NULL; -} - -offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1); -lqspi_load_cache(opaque, offset_within_the_region); -*size = LQSPI_CACHE_SIZE; -*offset = offset_within_the_region; -return q->lqspi_buf; -} - static uint64_t lqspi_read(void *opaque, hwaddr addr, unsigned int size) { @@ -1245,7 +1220,6 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) static const MemoryRegionOps lqspi_ops = { .read = lqspi_read, -.request_ptr = lqspi_request_mmio_ptr, .endianness = DEVICE_NATIVE_ENDIAN, .valid = { .min_access_size = 1, @@ -1322,15 +1296,6 @@ static void xilinx_qspips_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(sbd, &s->mmlqspi); q->lqspi_cached_addr = ~0ULL; - -/* mmio_execution breaks migration better aborting than having strange - * bugs. - */ -if (q->mmio_execution_enabled) { -error_setg(&q->migration_blocker, - "enabling mmio_execution breaks migration"); -migrate_add_blocker(q->migration_blocker, &error_fatal); -} } static void xlnx_zynqmp_qspips_realize(DeviceState *dev, Error **errp) @@ -1427,16 +1392,6 @@ static Property xilinx_zynqmp_qspips_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -static Property xilinx_qspips_properties[] = { -/* We had to turn this off for 2.10 as it is not compatible with migration. - * It can be enabled but will prevent the device to be migrated. - * This will go aways when a fix will be released. - */ -DEFINE_PROP_BOOL("x-mmio-exec", XilinxQSPIPS, mmio_execution_enabled, - false), -DEFINE_PROP_END_OF_LIST(), -}; - static Property xilinx_spips_properties[] = { DEFINE_PROP_UINT8("num-busses", XilinxSPIPS, num_busses, 1), DEFINE_PROP_UINT8("num-ss-bits", XilinxSPIPS, num_cs, 4), @@ -1450,7 +1405,6 @@ static void xilinx_qspips_class_init(ObjectClass *klass, void * data) XilinxSPIPSClass *xsc = XILINX_SPIPS_CLASS(klass); dc->realize = xilinx_qspips_realize; -dc->props = xilinx_qspips_properties; xsc->reg_ops = &qspips_ops; xsc->rx_fifo_size = RXFF_A_Q; xsc->tx_fifo_size = TXFF_A_Q; -- 2.18.0
Re: [Qemu-devel] [PATCH 1/3] hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
On Fri, Aug 17, 2018 at 4:46 AM, Peter Maydell wrote: > We now support direct execution from MMIO regions in the > core memory subsystem. This means that we don't need to > have device-specific support for it, and we can remove > the request_ptr handling from the Xilinx SPIPS device. > (It was broken anyway due to race conditions, and disabled > by default.) > > This device is the only in-tree user of this API. > > Signed-off-by: Peter Maydell Reviewed-by: Alistair Francis Alistair > --- > hw/ssi/xilinx_spips.c | 46 --- > 1 file changed, 46 deletions(-) > > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c > index c052bfc4b3c..16f88f74029 100644 > --- a/hw/ssi/xilinx_spips.c > +++ b/hw/ssi/xilinx_spips.c > @@ -1031,14 +1031,6 @@ static const MemoryRegionOps spips_ops = { > > static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q) > { > -XilinxSPIPS *s = &q->parent_obj; > - > -if ((q->mmio_execution_enabled) && (q->lqspi_cached_addr != ~0ULL)) { > -/* Invalidate the current mapped mmio */ > -memory_region_invalidate_mmio_ptr(&s->mmlqspi, q->lqspi_cached_addr, > - LQSPI_CACHE_SIZE); > -} > - > q->lqspi_cached_addr = ~0ULL; > } > > @@ -1207,23 +1199,6 @@ static void lqspi_load_cache(void *opaque, hwaddr addr) > } > } > > -static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned > *size, > -unsigned *offset) > -{ > -XilinxQSPIPS *q = opaque; > -hwaddr offset_within_the_region; > - > -if (!q->mmio_execution_enabled) { > -return NULL; > -} > - > -offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1); > -lqspi_load_cache(opaque, offset_within_the_region); > -*size = LQSPI_CACHE_SIZE; > -*offset = offset_within_the_region; > -return q->lqspi_buf; > -} > - > static uint64_t > lqspi_read(void *opaque, hwaddr addr, unsigned int size) > { > @@ -1245,7 +1220,6 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) > > static const MemoryRegionOps lqspi_ops = { > .read = lqspi_read, > -.request_ptr = lqspi_request_mmio_ptr, > .endianness = DEVICE_NATIVE_ENDIAN, > .valid = { > .min_access_size = 1, > @@ -1322,15 +1296,6 @@ static void xilinx_qspips_realize(DeviceState *dev, > Error **errp) > sysbus_init_mmio(sbd, &s->mmlqspi); > > q->lqspi_cached_addr = ~0ULL; > - > -/* mmio_execution breaks migration better aborting than having strange > - * bugs. > - */ > -if (q->mmio_execution_enabled) { > -error_setg(&q->migration_blocker, > - "enabling mmio_execution breaks migration"); > -migrate_add_blocker(q->migration_blocker, &error_fatal); > -} > } > > static void xlnx_zynqmp_qspips_realize(DeviceState *dev, Error **errp) > @@ -1427,16 +1392,6 @@ static Property xilinx_zynqmp_qspips_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > -static Property xilinx_qspips_properties[] = { > -/* We had to turn this off for 2.10 as it is not compatible with > migration. > - * It can be enabled but will prevent the device to be migrated. > - * This will go aways when a fix will be released. > - */ > -DEFINE_PROP_BOOL("x-mmio-exec", XilinxQSPIPS, mmio_execution_enabled, > - false), > -DEFINE_PROP_END_OF_LIST(), > -}; > - > static Property xilinx_spips_properties[] = { > DEFINE_PROP_UINT8("num-busses", XilinxSPIPS, num_busses, 1), > DEFINE_PROP_UINT8("num-ss-bits", XilinxSPIPS, num_cs, 4), > @@ -1450,7 +1405,6 @@ static void xilinx_qspips_class_init(ObjectClass > *klass, void * data) > XilinxSPIPSClass *xsc = XILINX_SPIPS_CLASS(klass); > > dc->realize = xilinx_qspips_realize; > -dc->props = xilinx_qspips_properties; > xsc->reg_ops = &qspips_ops; > xsc->rx_fifo_size = RXFF_A_Q; > xsc->tx_fifo_size = TXFF_A_Q; > -- > 2.18.0 > >
Re: [Qemu-devel] [PATCH 1/3] hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
Le 08/17/2018 à 01:46 PM, Peter Maydell a écrit : We now support direct execution from MMIO regions in the core memory subsystem. This means that we don't need to have device-specific support for it, and we can remove the request_ptr handling from the Xilinx SPIPS device. (It was broken anyway due to race conditions, and disabled by default.) This device is the only in-tree user of this API. Signed-off-by: Peter Maydell --- hw/ssi/xilinx_spips.c | 46 --- 1 file changed, 46 deletions(-) diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index c052bfc4b3c..16f88f74029 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -1031,14 +1031,6 @@ static const MemoryRegionOps spips_ops = { static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q) I'd drop this function and replace the calls by q->lqspi_cached_addr = ~0ULL; Otherwise: Reviewed-by: KONRAD Frederic { -XilinxSPIPS *s = &q->parent_obj; - -if ((q->mmio_execution_enabled) && (q->lqspi_cached_addr != ~0ULL)) { -/* Invalidate the current mapped mmio */ -memory_region_invalidate_mmio_ptr(&s->mmlqspi, q->lqspi_cached_addr, - LQSPI_CACHE_SIZE); -} - q->lqspi_cached_addr = ~0ULL; } @@ -1207,23 +1199,6 @@ static void lqspi_load_cache(void *opaque, hwaddr addr) } } -static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned *size, -unsigned *offset) -{ -XilinxQSPIPS *q = opaque; -hwaddr offset_within_the_region; - -if (!q->mmio_execution_enabled) { -return NULL; -} - -offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1); -lqspi_load_cache(opaque, offset_within_the_region); -*size = LQSPI_CACHE_SIZE; -*offset = offset_within_the_region; -return q->lqspi_buf; -} - static uint64_t lqspi_read(void *opaque, hwaddr addr, unsigned int size) { @@ -1245,7 +1220,6 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) static const MemoryRegionOps lqspi_ops = { .read = lqspi_read, -.request_ptr = lqspi_request_mmio_ptr, .endianness = DEVICE_NATIVE_ENDIAN, .valid = { .min_access_size = 1, @@ -1322,15 +1296,6 @@ static void xilinx_qspips_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(sbd, &s->mmlqspi); q->lqspi_cached_addr = ~0ULL; - -/* mmio_execution breaks migration better aborting than having strange - * bugs. - */ -if (q->mmio_execution_enabled) { -error_setg(&q->migration_blocker, - "enabling mmio_execution breaks migration"); -migrate_add_blocker(q->migration_blocker, &error_fatal); -} } static void xlnx_zynqmp_qspips_realize(DeviceState *dev, Error **errp) @@ -1427,16 +1392,6 @@ static Property xilinx_zynqmp_qspips_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -static Property xilinx_qspips_properties[] = { -/* We had to turn this off for 2.10 as it is not compatible with migration. - * It can be enabled but will prevent the device to be migrated. - * This will go aways when a fix will be released. - */ -DEFINE_PROP_BOOL("x-mmio-exec", XilinxQSPIPS, mmio_execution_enabled, - false), -DEFINE_PROP_END_OF_LIST(), -}; - static Property xilinx_spips_properties[] = { DEFINE_PROP_UINT8("num-busses", XilinxSPIPS, num_busses, 1), DEFINE_PROP_UINT8("num-ss-bits", XilinxSPIPS, num_cs, 4), @@ -1450,7 +1405,6 @@ static void xilinx_qspips_class_init(ObjectClass *klass, void * data) XilinxSPIPSClass *xsc = XILINX_SPIPS_CLASS(klass); dc->realize = xilinx_qspips_realize; -dc->props = xilinx_qspips_properties; xsc->reg_ops = &qspips_ops; xsc->rx_fifo_size = RXFF_A_Q; xsc->tx_fifo_size = TXFF_A_Q;
Re: [Qemu-devel] [PATCH 1/3] hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
On 20 August 2018 at 09:49, KONRAD Frederic wrote: > > > Le 08/17/2018 à 01:46 PM, Peter Maydell a écrit : >> >> We now support direct execution from MMIO regions in the >> core memory subsystem. This means that we don't need to >> have device-specific support for it, and we can remove >> the request_ptr handling from the Xilinx SPIPS device. >> (It was broken anyway due to race conditions, and disabled >> by default.) >> >> This device is the only in-tree user of this API. >> >> Signed-off-by: Peter Maydell >> --- >> hw/ssi/xilinx_spips.c | 46 --- >> 1 file changed, 46 deletions(-) >> >> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c >> index c052bfc4b3c..16f88f74029 100644 >> --- a/hw/ssi/xilinx_spips.c >> +++ b/hw/ssi/xilinx_spips.c >> @@ -1031,14 +1031,6 @@ static const MemoryRegionOps spips_ops = { >> static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q) > > > I'd drop this function and replace the calls by > q->lqspi_cached_addr = ~0ULL; I thought about that but decided it was reasonable to keep the function, because it matches with having a lqspi_load_cache() function still. thanks -- PMM
Re: [Qemu-devel] [PATCH 1/3] hw/ssi/xilinx_spips: Remove unneeded MMIO request_ptr code
Le 08/20/2018 à 10:53 AM, Peter Maydell a écrit : On 20 August 2018 at 09:49, KONRAD Frederic wrote: Le 08/17/2018 à 01:46 PM, Peter Maydell a écrit : We now support direct execution from MMIO regions in the core memory subsystem. This means that we don't need to have device-specific support for it, and we can remove the request_ptr handling from the Xilinx SPIPS device. (It was broken anyway due to race conditions, and disabled by default.) This device is the only in-tree user of this API. Signed-off-by: Peter Maydell --- hw/ssi/xilinx_spips.c | 46 --- 1 file changed, 46 deletions(-) diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c index c052bfc4b3c..16f88f74029 100644 --- a/hw/ssi/xilinx_spips.c +++ b/hw/ssi/xilinx_spips.c @@ -1031,14 +1031,6 @@ static const MemoryRegionOps spips_ops = { static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q) I'd drop this function and replace the calls by q->lqspi_cached_addr = ~0ULL; I thought about that but decided it was reasonable to keep the function, because it matches with having a lqspi_load_cache() function still. Right ok. Thanks, Fred thanks -- PMM