| Message ID | 20240402014647.3733839-1-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] CXL/cxl_type3: add first_dvsec_offset() helper | expand |
On Tue, Apr 02, 2024 at 09:46:46AM +0800, Li Zhijian via wrote: > It helps to figure out where the first dvsec register is located. In > addition, replace offset and size hardcore with existing macros. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > hw/mem/cxl_type3.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > index b0a7e9f11b64..ad2fe7d463fb 100644 > --- a/hw/mem/cxl_type3.c > +++ b/hw/mem/cxl_type3.c > @@ -643,6 +643,16 @@ static DOEProtocol doe_cdat_prot[] = { > { } > }; > > +static uint16_t first_dvsec_offset(CXLType3Dev *ct3d) > +{ > + uint16_t offset = PCI_CONFIG_SPACE_SIZE; > + > + if (ct3d->sn != UI64_NULL) > + offset += PCI_EXT_CAP_DSN_SIZEOF; > + > + return offset; > +} > + > static void ct3_realize(PCIDevice *pci_dev, Error **errp) > { > ERRP_GUARD(); > @@ -663,13 +673,10 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) > pci_config_set_prog_interface(pci_conf, 0x10); > > pcie_endpoint_cap_init(pci_dev, 0x80); > - if (ct3d->sn != UI64_NULL) { > - pcie_dev_ser_num_init(pci_dev, 0x100, ct3d->sn); > - cxl_cstate->dvsec_offset = 0x100 + 0x0c; > - } else { > - cxl_cstate->dvsec_offset = 0x100; > - } > + if (ct3d->sn != UI64_NULL) > + pcie_dev_ser_num_init(pci_dev, PCI_CONFIG_SPACE_SIZE, ct3d->sn); > > + cxl_cstate->dvsec_offset = first_dvsec_offset(ct3d); > ct3d->cxl_cstate.pdev = pci_dev; > build_dvsecs(ct3d); > > -- > 2.29.2 > > Hi Zhijian, Please use Qemu's checkpatch tool to make sure the patches meet the qemu code format requirement. Also, please cc linux-cxl@vger.kernel.org if the code is cxl related. Fan
On 02/04/2024 12:09, fan wrote: > On Tue, Apr 02, 2024 at 09:46:46AM +0800, Li Zhijian via wrote: >> It helps to figure out where the first dvsec register is located. In >> addition, replace offset and size hardcore with existing macros. >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> hw/mem/cxl_type3.c | 19 +++++++++++++------ >> 1 file changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c >> index b0a7e9f11b64..ad2fe7d463fb 100644 >> --- a/hw/mem/cxl_type3.c >> +++ b/hw/mem/cxl_type3.c >> @@ -643,6 +643,16 @@ static DOEProtocol doe_cdat_prot[] = { >> { } >> }; >> >> +static uint16_t first_dvsec_offset(CXLType3Dev *ct3d) >> +{ >> + uint16_t offset = PCI_CONFIG_SPACE_SIZE; >> + >> + if (ct3d->sn != UI64_NULL) >> + offset += PCI_EXT_CAP_DSN_SIZEOF; >> + >> + return offset; >> +} >> + >> static void ct3_realize(PCIDevice *pci_dev, Error **errp) >> { >> ERRP_GUARD(); >> @@ -663,13 +673,10 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) >> pci_config_set_prog_interface(pci_conf, 0x10); >> >> pcie_endpoint_cap_init(pci_dev, 0x80); >> - if (ct3d->sn != UI64_NULL) { >> - pcie_dev_ser_num_init(pci_dev, 0x100, ct3d->sn); >> - cxl_cstate->dvsec_offset = 0x100 + 0x0c; >> - } else { >> - cxl_cstate->dvsec_offset = 0x100; >> - } >> + if (ct3d->sn != UI64_NULL) >> + pcie_dev_ser_num_init(pci_dev, PCI_CONFIG_SPACE_SIZE, ct3d->sn); >> >> + cxl_cstate->dvsec_offset = first_dvsec_offset(ct3d); >> ct3d->cxl_cstate.pdev = pci_dev; >> build_dvsecs(ct3d); >> >> -- >> 2.29.2 >> >> > Hi Zhijian, > > Please use Qemu's checkpatch tool to make sure the patches meet the > qemu code format requirement. My mistake, any other input for these 2 patches? > Also, please cc linux-cxl@vger.kernel.org if the code is cxl related. Thanks for your remainder, do you mind if I send a patch to add the "L: linux-cxl@vger.kernel.org' field to the CXL entry Thanks Zhijian > > Fan
On Tue, 2 Apr 2024 09:46:46 +0800 Li Zhijian <lizhijian@fujitsu.com> wrote: > It helps to figure out where the first dvsec register is located. In > addition, replace offset and size hardcore with existing macros. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> I agree we should be using the macros. The offset calc is a bit specific to the the chosen memory layout, so not sure it makes sense to break it out to a separate function. I'll suggest alternative possible approaches in review of next patch. Jonathan > --- > hw/mem/cxl_type3.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > index b0a7e9f11b64..ad2fe7d463fb 100644 > --- a/hw/mem/cxl_type3.c > +++ b/hw/mem/cxl_type3.c > @@ -643,6 +643,16 @@ static DOEProtocol doe_cdat_prot[] = { > { } > }; > > +static uint16_t first_dvsec_offset(CXLType3Dev *ct3d) > +{ > + uint16_t offset = PCI_CONFIG_SPACE_SIZE; > + > + if (ct3d->sn != UI64_NULL) > + offset += PCI_EXT_CAP_DSN_SIZEOF; > + > + return offset; > +} > + > static void ct3_realize(PCIDevice *pci_dev, Error **errp) > { > ERRP_GUARD(); > @@ -663,13 +673,10 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) > pci_config_set_prog_interface(pci_conf, 0x10); > > pcie_endpoint_cap_init(pci_dev, 0x80); > - if (ct3d->sn != UI64_NULL) { > - pcie_dev_ser_num_init(pci_dev, 0x100, ct3d->sn); > - cxl_cstate->dvsec_offset = 0x100 + 0x0c; > - } else { > - cxl_cstate->dvsec_offset = 0x100; > - } > + if (ct3d->sn != UI64_NULL) > + pcie_dev_ser_num_init(pci_dev, PCI_CONFIG_SPACE_SIZE, ct3d->sn); > > + cxl_cstate->dvsec_offset = first_dvsec_offset(ct3d); > ct3d->cxl_cstate.pdev = pci_dev; > build_dvsecs(ct3d); >
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index b0a7e9f11b64..ad2fe7d463fb 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -643,6 +643,16 @@ static DOEProtocol doe_cdat_prot[] = { { } }; +static uint16_t first_dvsec_offset(CXLType3Dev *ct3d) +{ + uint16_t offset = PCI_CONFIG_SPACE_SIZE; + + if (ct3d->sn != UI64_NULL) + offset += PCI_EXT_CAP_DSN_SIZEOF; + + return offset; +} + static void ct3_realize(PCIDevice *pci_dev, Error **errp) { ERRP_GUARD(); @@ -663,13 +673,10 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) pci_config_set_prog_interface(pci_conf, 0x10); pcie_endpoint_cap_init(pci_dev, 0x80); - if (ct3d->sn != UI64_NULL) { - pcie_dev_ser_num_init(pci_dev, 0x100, ct3d->sn); - cxl_cstate->dvsec_offset = 0x100 + 0x0c; - } else { - cxl_cstate->dvsec_offset = 0x100; - } + if (ct3d->sn != UI64_NULL) + pcie_dev_ser_num_init(pci_dev, PCI_CONFIG_SPACE_SIZE, ct3d->sn); + cxl_cstate->dvsec_offset = first_dvsec_offset(ct3d); ct3d->cxl_cstate.pdev = pci_dev; build_dvsecs(ct3d);
It helps to figure out where the first dvsec register is located. In addition, replace offset and size hardcore with existing macros. Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- hw/mem/cxl_type3.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)