| Message ID | 20180817114619.22354-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Drop obsolete memory system request_ptr API | expand |
On Fri, Aug 17, 2018 at 4:46 AM, Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> 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 > >
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 <peter.maydell@linaro.org> > --- > 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 <frederic.konrad@adacore.com> > { > - 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; >
On 20 August 2018 at 09:49, KONRAD Frederic <frederic.konrad@adacore.com> 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 <peter.maydell@linaro.org> >> --- >> 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
Le 08/20/2018 à 10:53 AM, Peter Maydell a écrit : > On 20 August 2018 at 09:49, KONRAD Frederic <frederic.konrad@adacore.com> 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 <peter.maydell@linaro.org> >>> --- >>> 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 >
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;
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 <peter.maydell@linaro.org> --- hw/ssi/xilinx_spips.c | 46 ------------------------------------------- 1 file changed, 46 deletions(-) -- 2.18.0