Re: [PATCH 2/2] hw/cxl: Allow tracing component I/O accesses

[Jonathan Cameron via <qemu-devel@nongnu.org>]

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