* [PATCH 0/2] hw/cxl: Add tracing for component I/O region @ 2025-01-22 6:56 Philippe Mathieu-Daudé 2025-01-22 6:56 ` [PATCH 1/2] hw/cxl: Remove unused component_registers::io_registers[] array Philippe Mathieu-Daudé 2025-01-22 6:56 ` [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses Philippe Mathieu-Daudé 0 siblings, 2 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2025-01-22 6:56 UTC (permalink / raw) To: qemu-devel Cc: Fan Ni, Jonathan Cameron, Paolo Bonzini, Philippe Mathieu-Daudé Allow tracing component I/O accesses, remove unused io_registers[] array. Philippe Mathieu-Daudé (2): hw/cxl: Remove unused component_registers::io_registers[] array hw/cxl: Allow tracing component I/O accesses include/hw/cxl/cxl_component.h | 4 ++-- hw/cxl/cxl-component-utils.c | 14 +++++++++++--- hw/cxl/Kconfig | 1 + 3 files changed, 14 insertions(+), 5 deletions(-) -- 2.47.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] hw/cxl: Remove unused component_registers::io_registers[] array 2025-01-22 6:56 [PATCH 0/2] hw/cxl: Add tracing for component I/O region Philippe Mathieu-Daudé @ 2025-01-22 6:56 ` Philippe Mathieu-Daudé 2025-01-23 9:49 ` Jonathan Cameron via 2025-01-22 6:56 ` [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses Philippe Mathieu-Daudé 1 sibling, 1 reply; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2025-01-22 6:56 UTC (permalink / raw) To: qemu-devel Cc: Fan Ni, Jonathan Cameron, Paolo Bonzini, Philippe Mathieu-Daudé Avoid wasting 4K for each component, remove unused io_registers[]. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/cxl/cxl_component.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index 945ee6ffd04..ac61c3f33a5 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -218,7 +218,6 @@ typedef struct component_registers { * 0xe000 - 0xe3ff CXL ARB/MUX registers * 0xe400 - 0xffff RSVD */ - uint32_t io_registers[CXL2_COMPONENT_IO_REGION_SIZE >> 2]; MemoryRegion io; uint32_t cache_mem_registers[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; -- 2.47.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] hw/cxl: Remove unused component_registers::io_registers[] array 2025-01-22 6:56 ` [PATCH 1/2] hw/cxl: Remove unused component_registers::io_registers[] array Philippe Mathieu-Daudé @ 2025-01-23 9:49 ` Jonathan Cameron via 0 siblings, 0 replies; 8+ messages in thread From: Jonathan Cameron via @ 2025-01-23 9:49 UTC (permalink / raw) To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Fan Ni, Paolo Bonzini, mst On Wed, 22 Jan 2025 07:56:23 +0100 Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > Avoid wasting 4K for each component, remove unused io_registers[]. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> These may need to come back for correctness if anyone starts actually accessing them in the OS, but for now this is fine. We'd have to deal with filling them with plausible values anyway if that becomes an issue. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> I can pick these up and send on in a set to mst, or mst pick up directly, or got through another tree. Don't mind! > --- > include/hw/cxl/cxl_component.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h > index 945ee6ffd04..ac61c3f33a5 100644 > --- a/include/hw/cxl/cxl_component.h > +++ b/include/hw/cxl/cxl_component.h > @@ -218,7 +218,6 @@ typedef struct component_registers { > * 0xe000 - 0xe3ff CXL ARB/MUX registers > * 0xe400 - 0xffff RSVD > */ > - uint32_t io_registers[CXL2_COMPONENT_IO_REGION_SIZE >> 2]; > MemoryRegion io; > > uint32_t cache_mem_registers[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses 2025-01-22 6:56 [PATCH 0/2] hw/cxl: Add tracing for component I/O region Philippe Mathieu-Daudé 2025-01-22 6:56 ` [PATCH 1/2] hw/cxl: Remove unused component_registers::io_registers[] array Philippe Mathieu-Daudé @ 2025-01-22 6:56 ` Philippe Mathieu-Daudé 2025-01-23 9:51 ` Jonathan Cameron via 1 sibling, 1 reply; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2025-01-22 6:56 UTC (permalink / raw) To: qemu-devel Cc: Fan Ni, Jonathan Cameron, Paolo Bonzini, Philippe Mathieu-Daudé Map the component I/O region as UnimplementedDevice to be able to trace guest I/O accesses with '-d unimp'. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/cxl/cxl_component.h | 3 ++- hw/cxl/cxl-component-utils.c | 14 +++++++++++--- hw/cxl/Kconfig | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index ac61c3f33a5..54fd369a838 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -18,6 +18,7 @@ #include "qemu/range.h" #include "hw/cxl/cxl_cdat.h" #include "hw/register.h" +#include "hw/misc/unimp.h" #include "qapi/error.h" enum reg_type { @@ -218,7 +219,7 @@ typedef struct component_registers { * 0xe000 - 0xe3ff CXL ARB/MUX registers * 0xe400 - 0xffff RSVD */ - MemoryRegion io; + UnimplementedDeviceState io; uint32_t cache_mem_registers[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; uint32_t cache_mem_regs_write_mask[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c index cd116c04012..6d593475d00 100644 --- a/hw/cxl/cxl-component-utils.c +++ b/hw/cxl/cxl-component-utils.c @@ -192,17 +192,25 @@ void cxl_component_register_block_init(Object *obj, const char *type) { ComponentRegisters *cregs = &cxl_cstate->crb; + DeviceState *io_dev; + SysBusDevice *io_sbd; memory_region_init(&cregs->component_registers, obj, type, CXL2_COMPONENT_BLOCK_SIZE); /* io registers controls link which we don't care about in QEMU */ - memory_region_init_io(&cregs->io, obj, NULL, NULL, ".io", - CXL2_COMPONENT_IO_REGION_SIZE); + object_initialize_child(obj, "io", &cregs->io, TYPE_UNIMPLEMENTED_DEVICE); + io_dev = DEVICE(&cregs->io); + io_sbd = SYS_BUS_DEVICE(&cregs->io); + qdev_prop_set_string(io_dev, "name", ".io"); + qdev_prop_set_uint64(io_dev, "size", CXL2_COMPONENT_IO_REGION_SIZE); + sysbus_realize(io_sbd, &error_fatal); + memory_region_init_io(&cregs->cache_mem, obj, &cache_mem_ops, cxl_cstate, ".cache_mem", CXL2_COMPONENT_CM_REGION_SIZE); - memory_region_add_subregion(&cregs->component_registers, 0, &cregs->io); + memory_region_add_subregion(&cregs->component_registers, 0, + sysbus_mmio_get_region(io_sbd, 0)); memory_region_add_subregion(&cregs->component_registers, CXL2_COMPONENT_IO_REGION_SIZE, &cregs->cache_mem); diff --git a/hw/cxl/Kconfig b/hw/cxl/Kconfig index 8e67519b161..d6c7536001e 100644 --- a/hw/cxl/Kconfig +++ b/hw/cxl/Kconfig @@ -1,3 +1,4 @@ config CXL bool default y if PCI_EXPRESS + select UNIMP -- 2.47.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses 2025-01-22 6:56 ` [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses Philippe Mathieu-Daudé @ 2025-01-23 9:51 ` Jonathan Cameron via 2025-01-24 16:20 ` Jonathan Cameron via 0 siblings, 1 reply; 8+ messages in thread From: Jonathan Cameron via @ 2025-01-23 9:51 UTC (permalink / raw) To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Fan Ni, Paolo Bonzini, mst On Wed, 22 Jan 2025 07:56:24 +0100 Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > Map the component I/O region as UnimplementedDevice > to be able to trace guest I/O accesses with '-d unimp'. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> I'm not that familiar with this infrastructure but seems fine to me. I'd definitely be curious if anything is touching this space so tracing may be helpful for that! Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > include/hw/cxl/cxl_component.h | 3 ++- > hw/cxl/cxl-component-utils.c | 14 +++++++++++--- > hw/cxl/Kconfig | 1 + > 3 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h > index ac61c3f33a5..54fd369a838 100644 > --- a/include/hw/cxl/cxl_component.h > +++ b/include/hw/cxl/cxl_component.h > @@ -18,6 +18,7 @@ > #include "qemu/range.h" > #include "hw/cxl/cxl_cdat.h" > #include "hw/register.h" > +#include "hw/misc/unimp.h" > #include "qapi/error.h" > > enum reg_type { > @@ -218,7 +219,7 @@ typedef struct component_registers { > * 0xe000 - 0xe3ff CXL ARB/MUX registers > * 0xe400 - 0xffff RSVD > */ > - MemoryRegion io; > + UnimplementedDeviceState io; > > uint32_t cache_mem_registers[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; > uint32_t cache_mem_regs_write_mask[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c > index cd116c04012..6d593475d00 100644 > --- a/hw/cxl/cxl-component-utils.c > +++ b/hw/cxl/cxl-component-utils.c > @@ -192,17 +192,25 @@ void cxl_component_register_block_init(Object *obj, > const char *type) > { > ComponentRegisters *cregs = &cxl_cstate->crb; > + DeviceState *io_dev; > + SysBusDevice *io_sbd; > > memory_region_init(&cregs->component_registers, obj, type, > CXL2_COMPONENT_BLOCK_SIZE); > > /* io registers controls link which we don't care about in QEMU */ > - memory_region_init_io(&cregs->io, obj, NULL, NULL, ".io", > - CXL2_COMPONENT_IO_REGION_SIZE); > + object_initialize_child(obj, "io", &cregs->io, TYPE_UNIMPLEMENTED_DEVICE); > + io_dev = DEVICE(&cregs->io); > + io_sbd = SYS_BUS_DEVICE(&cregs->io); > + qdev_prop_set_string(io_dev, "name", ".io"); > + qdev_prop_set_uint64(io_dev, "size", CXL2_COMPONENT_IO_REGION_SIZE); > + sysbus_realize(io_sbd, &error_fatal); > + > memory_region_init_io(&cregs->cache_mem, obj, &cache_mem_ops, cxl_cstate, > ".cache_mem", CXL2_COMPONENT_CM_REGION_SIZE); > > - memory_region_add_subregion(&cregs->component_registers, 0, &cregs->io); > + memory_region_add_subregion(&cregs->component_registers, 0, > + sysbus_mmio_get_region(io_sbd, 0)); > memory_region_add_subregion(&cregs->component_registers, > CXL2_COMPONENT_IO_REGION_SIZE, > &cregs->cache_mem); > diff --git a/hw/cxl/Kconfig b/hw/cxl/Kconfig > index 8e67519b161..d6c7536001e 100644 > --- a/hw/cxl/Kconfig > +++ b/hw/cxl/Kconfig > @@ -1,3 +1,4 @@ > config CXL > bool > default y if PCI_EXPRESS > + select UNIMP ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses 2025-01-23 9:51 ` Jonathan Cameron via @ 2025-01-24 16:20 ` Jonathan Cameron via 2025-01-24 16:28 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 8+ messages in thread From: Jonathan Cameron via @ 2025-01-24 16:20 UTC (permalink / raw) To: Jonathan Cameron via Cc: Jonathan Cameron, Philippe Mathieu-Daudé, Fan Ni, Paolo Bonzini, mst On Thu, 23 Jan 2025 09:51:51 +0000 Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > On Wed, 22 Jan 2025 07:56:24 +0100 > Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > > Map the component I/O region as UnimplementedDevice > > to be able to trace guest I/O accesses with '-d unimp'. > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > I'm not that familiar with this infrastructure but seems > fine to me. > > I'd definitely be curious if anything is touching this space so > tracing may be helpful for that! Hi Philippe > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Retract that. Can't instantiate a CXL device with this patch because: Device unimplemented-device can not be dynamically instantiated. Reverting this patch on my tree fixes that. > > --- > > include/hw/cxl/cxl_component.h | 3 ++- > > hw/cxl/cxl-component-utils.c | 14 +++++++++++--- > > hw/cxl/Kconfig | 1 + > > 3 files changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h > > index ac61c3f33a5..54fd369a838 100644 > > --- a/include/hw/cxl/cxl_component.h > > +++ b/include/hw/cxl/cxl_component.h > > @@ -18,6 +18,7 @@ > > #include "qemu/range.h" > > #include "hw/cxl/cxl_cdat.h" > > #include "hw/register.h" > > +#include "hw/misc/unimp.h" > > #include "qapi/error.h" > > > > enum reg_type { > > @@ -218,7 +219,7 @@ typedef struct component_registers { > > * 0xe000 - 0xe3ff CXL ARB/MUX registers > > * 0xe400 - 0xffff RSVD > > */ > > - MemoryRegion io; > > + UnimplementedDeviceState io; > > > > uint32_t cache_mem_registers[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; > > uint32_t cache_mem_regs_write_mask[CXL2_COMPONENT_CM_REGION_SIZE >> 2]; > > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c > > index cd116c04012..6d593475d00 100644 > > --- a/hw/cxl/cxl-component-utils.c > > +++ b/hw/cxl/cxl-component-utils.c > > @@ -192,17 +192,25 @@ void cxl_component_register_block_init(Object *obj, > > const char *type) > > { > > ComponentRegisters *cregs = &cxl_cstate->crb; > > + DeviceState *io_dev; > > + SysBusDevice *io_sbd; > > > > memory_region_init(&cregs->component_registers, obj, type, > > CXL2_COMPONENT_BLOCK_SIZE); > > > > /* io registers controls link which we don't care about in QEMU */ > > - memory_region_init_io(&cregs->io, obj, NULL, NULL, ".io", > > - CXL2_COMPONENT_IO_REGION_SIZE); > > + object_initialize_child(obj, "io", &cregs->io, TYPE_UNIMPLEMENTED_DEVICE); > > + io_dev = DEVICE(&cregs->io); > > + io_sbd = SYS_BUS_DEVICE(&cregs->io); > > + qdev_prop_set_string(io_dev, "name", ".io"); > > + qdev_prop_set_uint64(io_dev, "size", CXL2_COMPONENT_IO_REGION_SIZE); > > + sysbus_realize(io_sbd, &error_fatal); > > + > > memory_region_init_io(&cregs->cache_mem, obj, &cache_mem_ops, cxl_cstate, > > ".cache_mem", CXL2_COMPONENT_CM_REGION_SIZE); > > > > - memory_region_add_subregion(&cregs->component_registers, 0, &cregs->io); > > + memory_region_add_subregion(&cregs->component_registers, 0, > > + sysbus_mmio_get_region(io_sbd, 0)); > > memory_region_add_subregion(&cregs->component_registers, > > CXL2_COMPONENT_IO_REGION_SIZE, > > &cregs->cache_mem); > > diff --git a/hw/cxl/Kconfig b/hw/cxl/Kconfig > > index 8e67519b161..d6c7536001e 100644 > > --- a/hw/cxl/Kconfig > > +++ b/hw/cxl/Kconfig > > @@ -1,3 +1,4 @@ > > config CXL > > bool > > default y if PCI_EXPRESS > > + select UNIMP > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses 2025-01-24 16:20 ` Jonathan Cameron via @ 2025-01-24 16:28 ` Philippe Mathieu-Daudé 2025-01-31 19:07 ` Jonathan Cameron via 0 siblings, 1 reply; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2025-01-24 16:28 UTC (permalink / raw) To: Jonathan Cameron, Jonathan Cameron via; +Cc: Fan Ni, Paolo Bonzini, mst On 24/1/25 17:20, Jonathan Cameron wrote: > On Thu, 23 Jan 2025 09:51:51 +0000 > Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > >> On Wed, 22 Jan 2025 07:56:24 +0100 >> Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >>> Map the component I/O region as UnimplementedDevice >>> to be able to trace guest I/O accesses with '-d unimp'. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> I'm not that familiar with this infrastructure but seems >> fine to me. >> >> I'd definitely be curious if anything is touching this space so >> tracing may be helpful for that! > Hi Philippe > >> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Retract that. > > Can't instantiate a CXL device with this patch because: > > Device unimplemented-device can not be dynamically instantiated. Hmm the qtests using CXL devices pass, how do you trigger that? > > Reverting this patch on my tree fixes that. > >>> --- >>> include/hw/cxl/cxl_component.h | 3 ++- >>> hw/cxl/cxl-component-utils.c | 14 +++++++++++--- >>> hw/cxl/Kconfig | 1 + >>> 3 files changed, 14 insertions(+), 4 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses 2025-01-24 16:28 ` Philippe Mathieu-Daudé @ 2025-01-31 19:07 ` Jonathan Cameron via 0 siblings, 0 replies; 8+ messages in thread From: Jonathan Cameron via @ 2025-01-31 19:07 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Jonathan Cameron via, Fan Ni, Paolo Bonzini, mst On Fri, 24 Jan 2025 17:28:02 +0100 Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > On 24/1/25 17:20, Jonathan Cameron wrote: > > On Thu, 23 Jan 2025 09:51:51 +0000 > > Jonathan Cameron via <qemu-devel@nongnu.org> wrote: > > > >> On Wed, 22 Jan 2025 07:56:24 +0100 > >> Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > >> > >>> Map the component I/O region as UnimplementedDevice > >>> to be able to trace guest I/O accesses with '-d unimp'. > >>> > >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > >> I'm not that familiar with this infrastructure but seems > >> fine to me. > >> > >> I'd definitely be curious if anything is touching this space so > >> tracing may be helpful for that! > > Hi Philippe > > > >> > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Retract that. > > > > Can't instantiate a CXL device with this patch because: > > > > Device unimplemented-device can not be dynamically instantiated. > > Hmm the qtests using CXL devices pass, how do you trigger that? Seems oddly enough to be arm64 only (well not x86 - there is no support for other arches than those two) which isn't upstream yet hence no tests upstream - I'll get back to that sometime soonish as the blocker of dt-bindings for PXBs may be less of an issue than it was. source of that print seems to be hw/core/sysbus-fdt.c add_fdt_node(). Seems it is looking for sysbus devices and gets confused when it finds this one. I'm not sure if it is in /peripheral or /peripheral-anon. For now you could try my gitlab tree gitlab.com/jic23/qemu.git cxl-* whatever has latest date. Or shout if you want me to try anything. Jonathan > > > > > Reverting this patch on my tree fixes that. > > > >>> --- > >>> include/hw/cxl/cxl_component.h | 3 ++- > >>> hw/cxl/cxl-component-utils.c | 14 +++++++++++--- > >>> hw/cxl/Kconfig | 1 + > >>> 3 files changed, 14 insertions(+), 4 deletions(-) > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-01-31 19:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-22 6:56 [PATCH 0/2] hw/cxl: Add tracing for component I/O region Philippe Mathieu-Daudé 2025-01-22 6:56 ` [PATCH 1/2] hw/cxl: Remove unused component_registers::io_registers[] array Philippe Mathieu-Daudé 2025-01-23 9:49 ` Jonathan Cameron via 2025-01-22 6:56 ` [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses Philippe Mathieu-Daudé 2025-01-23 9:51 ` Jonathan Cameron via 2025-01-24 16:20 ` Jonathan Cameron via 2025-01-24 16:28 ` Philippe Mathieu-Daudé 2025-01-31 19:07 ` Jonathan Cameron via
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).