* [PATCH qemu] hw/pci-bridge/pxb-cxl: Drop RAS capability from host bridge.
@ 2024-02-15 15:52 Jonathan Cameron via
2024-02-15 16:11 ` Michael S. Tsirkin
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron via @ 2024-02-15 15:52 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Michael Tsirkin; +Cc: linuxarm, linux-cxl
This CXL component isn't allowed to have a RAS capability.
Whilst this should be harmless as software is not expected to look
here, good to clean it up.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
I've been carrying this on my tree for quite a while.
This wasn't in previous fixes set because it's low priority and
rebasing it across cleanup series that followed those fixes was
too fiddly to bother.
include/hw/cxl/cxl_component.h | 1 +
hw/cxl/cxl-component-utils.c | 21 +++++++++++++++++----
hw/pci-bridge/pci_expander_bridge.c | 2 +-
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index 0e5d35c263..5012fab6f7 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -25,6 +25,7 @@ enum reg_type {
CXL2_TYPE3_DEVICE,
CXL2_LOGICAL_DEVICE,
CXL2_ROOT_PORT,
+ CXL2_RC,
CXL2_UPSTREAM_PORT,
CXL2_DOWNSTREAM_PORT,
CXL3_SWITCH_MAILBOX_CCI,
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 84ab503325..cd116c0401 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -297,6 +297,7 @@ void cxl_component_register_init_common(uint32_t *reg_state,
caps = 3;
break;
case CXL2_ROOT_PORT:
+ case CXL2_RC:
/* + Extended Security, + Snoop */
caps = 5;
break;
@@ -326,8 +327,19 @@ void cxl_component_register_init_common(uint32_t *reg_state,
CXL_##reg##_REGISTERS_OFFSET); \
} while (0)
+ switch (type) {
+ case CXL2_DEVICE:
+ case CXL2_TYPE3_DEVICE:
+ case CXL2_LOGICAL_DEVICE:
+ case CXL2_ROOT_PORT:
+ case CXL2_UPSTREAM_PORT:
+ case CXL2_DOWNSTREAM_PORT:
init_cap_reg(RAS, 2, CXL_RAS_CAPABILITY_VERSION);
- ras_init_common(reg_state, write_msk);
+ ras_init_common(reg_state, write_msk);
+ break;
+ default:
+ break;
+ }
init_cap_reg(LINK, 4, CXL_LINK_CAPABILITY_VERSION);
@@ -335,9 +347,10 @@ void cxl_component_register_init_common(uint32_t *reg_state,
return;
}
- init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
- hdm_init_common(reg_state, write_msk, type);
-
+ if (type != CXL2_ROOT_PORT) {
+ init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
+ hdm_init_common(reg_state, write_msk, type);
+ }
if (caps < 5) {
return;
}
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 535889f7c2..0411ad31ea 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -290,7 +290,7 @@ static void pxb_cxl_dev_reset(DeviceState *dev)
uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
int dsp_count = 0;
- cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
+ cxl_component_register_init_common(reg_state, write_msk, CXL2_RC);
/*
* The CXL specification allows for host bridges with no HDM decoders
* if they only have a single root port.
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH qemu] hw/pci-bridge/pxb-cxl: Drop RAS capability from host bridge.
2024-02-15 15:52 [PATCH qemu] hw/pci-bridge/pxb-cxl: Drop RAS capability from host bridge Jonathan Cameron via
@ 2024-02-15 16:11 ` Michael S. Tsirkin
2024-02-15 17:42 ` Jonathan Cameron via
0 siblings, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2024-02-15 16:11 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: qemu-devel, Fan Ni, linuxarm, linux-cxl
On Thu, Feb 15, 2024 at 03:52:06PM +0000, Jonathan Cameron wrote:
> This CXL component isn't allowed to have a RAS capability.
> Whilst this should be harmless as software is not expected to look
> here, good to clean it up.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Isn't this device migrateable? If yes you need compat
handling.
> ---
> I've been carrying this on my tree for quite a while.
> This wasn't in previous fixes set because it's low priority and
> rebasing it across cleanup series that followed those fixes was
> too fiddly to bother.
>
> include/hw/cxl/cxl_component.h | 1 +
> hw/cxl/cxl-component-utils.c | 21 +++++++++++++++++----
> hw/pci-bridge/pci_expander_bridge.c | 2 +-
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> index 0e5d35c263..5012fab6f7 100644
> --- a/include/hw/cxl/cxl_component.h
> +++ b/include/hw/cxl/cxl_component.h
> @@ -25,6 +25,7 @@ enum reg_type {
> CXL2_TYPE3_DEVICE,
> CXL2_LOGICAL_DEVICE,
> CXL2_ROOT_PORT,
> + CXL2_RC,
> CXL2_UPSTREAM_PORT,
> CXL2_DOWNSTREAM_PORT,
> CXL3_SWITCH_MAILBOX_CCI,
> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> index 84ab503325..cd116c0401 100644
> --- a/hw/cxl/cxl-component-utils.c
> +++ b/hw/cxl/cxl-component-utils.c
> @@ -297,6 +297,7 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> caps = 3;
> break;
> case CXL2_ROOT_PORT:
> + case CXL2_RC:
> /* + Extended Security, + Snoop */
> caps = 5;
> break;
> @@ -326,8 +327,19 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> CXL_##reg##_REGISTERS_OFFSET); \
> } while (0)
>
> + switch (type) {
> + case CXL2_DEVICE:
> + case CXL2_TYPE3_DEVICE:
> + case CXL2_LOGICAL_DEVICE:
> + case CXL2_ROOT_PORT:
> + case CXL2_UPSTREAM_PORT:
> + case CXL2_DOWNSTREAM_PORT:
> init_cap_reg(RAS, 2, CXL_RAS_CAPABILITY_VERSION);
> - ras_init_common(reg_state, write_msk);
> + ras_init_common(reg_state, write_msk);
> + break;
> + default:
> + break;
> + }
>
> init_cap_reg(LINK, 4, CXL_LINK_CAPABILITY_VERSION);
>
> @@ -335,9 +347,10 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> return;
> }
>
> - init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
> - hdm_init_common(reg_state, write_msk, type);
> -
> + if (type != CXL2_ROOT_PORT) {
> + init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
> + hdm_init_common(reg_state, write_msk, type);
> + }
> if (caps < 5) {
> return;
> }
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 535889f7c2..0411ad31ea 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -290,7 +290,7 @@ static void pxb_cxl_dev_reset(DeviceState *dev)
> uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
> int dsp_count = 0;
>
> - cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
> + cxl_component_register_init_common(reg_state, write_msk, CXL2_RC);
> /*
> * The CXL specification allows for host bridges with no HDM decoders
> * if they only have a single root port.
> --
> 2.39.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH qemu] hw/pci-bridge/pxb-cxl: Drop RAS capability from host bridge.
2024-02-15 16:11 ` Michael S. Tsirkin
@ 2024-02-15 17:42 ` Jonathan Cameron via
2024-03-12 15:30 ` Michael S. Tsirkin
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron via @ 2024-02-15 17:42 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, Fan Ni, linuxarm, linux-cxl
On Thu, 15 Feb 2024 11:11:47 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Feb 15, 2024 at 03:52:06PM +0000, Jonathan Cameron wrote:
> > This CXL component isn't allowed to have a RAS capability.
> > Whilst this should be harmless as software is not expected to look
> > here, good to clean it up.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> Isn't this device migrateable? If yes you need compat
> handling.
Not yet. Migrating these is broken in lots of ways :(
Given they are functional emulation only I've never cared that much.
We'll need to fix this as part of adding support for virtualization
use cases which start to make sense when dynamic capacity
support lands in kernel + QEMU (probably later this year)
Jonathan
>
>
> > ---
> > I've been carrying this on my tree for quite a while.
> > This wasn't in previous fixes set because it's low priority and
> > rebasing it across cleanup series that followed those fixes was
> > too fiddly to bother.
> >
> > include/hw/cxl/cxl_component.h | 1 +
> > hw/cxl/cxl-component-utils.c | 21 +++++++++++++++++----
> > hw/pci-bridge/pci_expander_bridge.c | 2 +-
> > 3 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> > index 0e5d35c263..5012fab6f7 100644
> > --- a/include/hw/cxl/cxl_component.h
> > +++ b/include/hw/cxl/cxl_component.h
> > @@ -25,6 +25,7 @@ enum reg_type {
> > CXL2_TYPE3_DEVICE,
> > CXL2_LOGICAL_DEVICE,
> > CXL2_ROOT_PORT,
> > + CXL2_RC,
> > CXL2_UPSTREAM_PORT,
> > CXL2_DOWNSTREAM_PORT,
> > CXL3_SWITCH_MAILBOX_CCI,
> > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > index 84ab503325..cd116c0401 100644
> > --- a/hw/cxl/cxl-component-utils.c
> > +++ b/hw/cxl/cxl-component-utils.c
> > @@ -297,6 +297,7 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> > caps = 3;
> > break;
> > case CXL2_ROOT_PORT:
> > + case CXL2_RC:
> > /* + Extended Security, + Snoop */
> > caps = 5;
> > break;
> > @@ -326,8 +327,19 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> > CXL_##reg##_REGISTERS_OFFSET); \
> > } while (0)
> >
> > + switch (type) {
> > + case CXL2_DEVICE:
> > + case CXL2_TYPE3_DEVICE:
> > + case CXL2_LOGICAL_DEVICE:
> > + case CXL2_ROOT_PORT:
> > + case CXL2_UPSTREAM_PORT:
> > + case CXL2_DOWNSTREAM_PORT:
> > init_cap_reg(RAS, 2, CXL_RAS_CAPABILITY_VERSION);
> > - ras_init_common(reg_state, write_msk);
> > + ras_init_common(reg_state, write_msk);
> > + break;
> > + default:
> > + break;
> > + }
> >
> > init_cap_reg(LINK, 4, CXL_LINK_CAPABILITY_VERSION);
> >
> > @@ -335,9 +347,10 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> > return;
> > }
> >
> > - init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
> > - hdm_init_common(reg_state, write_msk, type);
> > -
> > + if (type != CXL2_ROOT_PORT) {
> > + init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
> > + hdm_init_common(reg_state, write_msk, type);
> > + }
> > if (caps < 5) {
> > return;
> > }
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > index 535889f7c2..0411ad31ea 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -290,7 +290,7 @@ static void pxb_cxl_dev_reset(DeviceState *dev)
> > uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
> > int dsp_count = 0;
> >
> > - cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
> > + cxl_component_register_init_common(reg_state, write_msk, CXL2_RC);
> > /*
> > * The CXL specification allows for host bridges with no HDM decoders
> > * if they only have a single root port.
> > --
> > 2.39.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH qemu] hw/pci-bridge/pxb-cxl: Drop RAS capability from host bridge.
2024-02-15 17:42 ` Jonathan Cameron via
@ 2024-03-12 15:30 ` Michael S. Tsirkin
0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2024-03-12 15:30 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: qemu-devel, Fan Ni, linuxarm, linux-cxl
On Thu, Feb 15, 2024 at 05:42:12PM +0000, Jonathan Cameron wrote:
> On Thu, 15 Feb 2024 11:11:47 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Thu, Feb 15, 2024 at 03:52:06PM +0000, Jonathan Cameron wrote:
> > > This CXL component isn't allowed to have a RAS capability.
> > > Whilst this should be harmless as software is not expected to look
> > > here, good to clean it up.
> > >
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Isn't this device migrateable? If yes you need compat
> > handling.
>
> Not yet. Migrating these is broken in lots of ways :(
>
> Given they are functional emulation only I've never cared that much.
> We'll need to fix this as part of adding support for virtualization
> use cases which start to make sense when dynamic capacity
> support lands in kernel + QEMU (probably later this year)
>
> Jonathan
should probably block migration then?
> >
> >
> > > ---
> > > I've been carrying this on my tree for quite a while.
> > > This wasn't in previous fixes set because it's low priority and
> > > rebasing it across cleanup series that followed those fixes was
> > > too fiddly to bother.
> > >
> > > include/hw/cxl/cxl_component.h | 1 +
> > > hw/cxl/cxl-component-utils.c | 21 +++++++++++++++++----
> > > hw/pci-bridge/pci_expander_bridge.c | 2 +-
> > > 3 files changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> > > index 0e5d35c263..5012fab6f7 100644
> > > --- a/include/hw/cxl/cxl_component.h
> > > +++ b/include/hw/cxl/cxl_component.h
> > > @@ -25,6 +25,7 @@ enum reg_type {
> > > CXL2_TYPE3_DEVICE,
> > > CXL2_LOGICAL_DEVICE,
> > > CXL2_ROOT_PORT,
> > > + CXL2_RC,
> > > CXL2_UPSTREAM_PORT,
> > > CXL2_DOWNSTREAM_PORT,
> > > CXL3_SWITCH_MAILBOX_CCI,
> > > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > > index 84ab503325..cd116c0401 100644
> > > --- a/hw/cxl/cxl-component-utils.c
> > > +++ b/hw/cxl/cxl-component-utils.c
> > > @@ -297,6 +297,7 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> > > caps = 3;
> > > break;
> > > case CXL2_ROOT_PORT:
> > > + case CXL2_RC:
> > > /* + Extended Security, + Snoop */
> > > caps = 5;
> > > break;
> > > @@ -326,8 +327,19 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> > > CXL_##reg##_REGISTERS_OFFSET); \
> > > } while (0)
> > >
> > > + switch (type) {
> > > + case CXL2_DEVICE:
> > > + case CXL2_TYPE3_DEVICE:
> > > + case CXL2_LOGICAL_DEVICE:
> > > + case CXL2_ROOT_PORT:
> > > + case CXL2_UPSTREAM_PORT:
> > > + case CXL2_DOWNSTREAM_PORT:
> > > init_cap_reg(RAS, 2, CXL_RAS_CAPABILITY_VERSION);
> > > - ras_init_common(reg_state, write_msk);
> > > + ras_init_common(reg_state, write_msk);
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > >
> > > init_cap_reg(LINK, 4, CXL_LINK_CAPABILITY_VERSION);
> > >
> > > @@ -335,9 +347,10 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> > > return;
> > > }
> > >
> > > - init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
> > > - hdm_init_common(reg_state, write_msk, type);
> > > -
> > > + if (type != CXL2_ROOT_PORT) {
> > > + init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
> > > + hdm_init_common(reg_state, write_msk, type);
> > > + }
> > > if (caps < 5) {
> > > return;
> > > }
> > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > > index 535889f7c2..0411ad31ea 100644
> > > --- a/hw/pci-bridge/pci_expander_bridge.c
> > > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > > @@ -290,7 +290,7 @@ static void pxb_cxl_dev_reset(DeviceState *dev)
> > > uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
> > > int dsp_count = 0;
> > >
> > > - cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
> > > + cxl_component_register_init_common(reg_state, write_msk, CXL2_RC);
> > > /*
> > > * The CXL specification allows for host bridges with no HDM decoders
> > > * if they only have a single root port.
> > > --
> > > 2.39.2
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-12 15:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-15 15:52 [PATCH qemu] hw/pci-bridge/pxb-cxl: Drop RAS capability from host bridge Jonathan Cameron via
2024-02-15 16:11 ` Michael S. Tsirkin
2024-02-15 17:42 ` Jonathan Cameron via
2024-03-12 15:30 ` Michael S. Tsirkin
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).