From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>,
Dan Williams <dan.j.williams@intel.com>,
qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [RFC PATCH 11/25] hw/pxb: Allow creation of a CXL PXB (host bridge)
Date: Mon, 16 Nov 2020 16:44:09 +0000 [thread overview]
Message-ID: <20201116164409.00005f3b@Huawei.com> (raw)
In-Reply-To: <20201111054724.794888-12-ben.widawsky@intel.com>
On Tue, 10 Nov 2020 21:47:10 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:
> This works like adding a typical pxb device, except the name is
> 'pxb-cxl' instead of 'pxb-pcie'. An example command line would be as
> follows:
> -device pxb-cxl,id=cxl.0,bus="pcie.0",bus_nr=1
>
> A CXL PXB is backward compatible with PCIe. What this means in practice
> is that an operating system that is unaware of CXL should still be able
> to enumerate this topology as if it were PCIe.
>
> One can create multiple CXL PXB host bridges, but a host bridge can only
> be connected to the main root bus. Host bridges cannot appear elsewhere
> in the topology.
>
> Note that as of this patch, the ACPI tables needed for the host bridge
> (specifically, an ACPI object in _SB named ACPI0016 and the CEDT) aren't
> created. So while this patch internally creates it, it cannot be
> properly used by an operating system or other system software.
>
> Upcoming patches will allow creating multiple host bridges.
>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
Hi Ben,
Few minor things inline.
Jonathan
> ---
> hw/pci-bridge/pci_expander_bridge.c | 67 ++++++++++++++++++++++++++++-
> hw/pci/pci.c | 7 +++
> include/hw/pci/pci.h | 6 +++
> 3 files changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 88c45dc3b5..3a8d815231 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -56,6 +56,10 @@ DECLARE_INSTANCE_CHECKER(PXBDev, PXB_DEV,
> DECLARE_INSTANCE_CHECKER(PXBDev, PXB_PCIE_DEV,
> TYPE_PXB_PCIE_DEVICE)
>
> +#define TYPE_PXB_CXL_DEVICE "pxb-cxl"
> +DECLARE_INSTANCE_CHECKER(PXBDev, PXB_CXL_DEV,
> + TYPE_PXB_CXL_DEVICE)
> +
> struct PXBDev {
> /*< private >*/
> PCIDevice parent_obj;
> @@ -67,6 +71,11 @@ struct PXBDev {
>
> static PXBDev *convert_to_pxb(PCIDevice *dev)
> {
> + /* A CXL PXB's parent bus is PCIe, so the normal check won't work */
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PXB_CXL_DEVICE)) {
> + return PXB_CXL_DEV(dev);
> + }
> +
> return pci_bus_is_express(pci_get_bus(dev))
> ? PXB_PCIE_DEV(dev) : PXB_DEV(dev);
> }
> @@ -111,11 +120,20 @@ static const TypeInfo pxb_pcie_bus_info = {
> .class_init = pxb_bus_class_init,
> };
>
> +static const TypeInfo pxb_cxl_bus_info = {
> + .name = TYPE_PXB_CXL_BUS,
> + .parent = TYPE_CXL_BUS,
> + .instance_size = sizeof(PXBBus),
> + .class_init = pxb_bus_class_init,
> +};
> +
> static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,
> PCIBus *rootbus)
> {
> - PXBBus *bus = pci_bus_is_express(rootbus) ?
> - PXB_PCIE_BUS(rootbus) : PXB_BUS(rootbus);
> + PXBBus *bus = pci_bus_is_cxl(rootbus) ?
> + PXB_CXL_BUS(rootbus) :
> + pci_bus_is_express(rootbus) ? PXB_PCIE_BUS(rootbus) :
> + PXB_BUS(rootbus);
There comes a point where if / else is much more readable.
>
> snprintf(bus->bus_path, 8, "0000:%02x", pxb_bus_num(rootbus));
> return bus->bus_path;
> @@ -380,13 +398,58 @@ static const TypeInfo pxb_pcie_dev_info = {
> },
> };
>
> +static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp)
> +{
> + /* A CXL PXB's parent bus is still PCIe */
> + if (!pci_bus_is_express(pci_get_bus(dev))) {
> + error_setg(errp, "pxb-cxl devices cannot reside on a PCI bus");
> + return;
> + }
> +
> + pxb_dev_realize_common(dev, CXL, errp);
> +}
> +
> +static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +
> + k->realize = pxb_cxl_dev_realize;
> + k->exit = pxb_dev_exitfn;
> + k->vendor_id = PCI_VENDOR_ID_INTEL;
> + k->device_id = 0xabcd;
Just to check, is that an officially assigned device_id that we will never
have a clash with? Nice ID to get if it is :)
> + k->class_id = PCI_CLASS_BRIDGE_HOST;
> + k->subsystem_vendor_id = PCI_VENDOR_ID_INTEL;
> +
> + dc->desc = "CXL Host Bridge";
> + device_class_set_props(dc, pxb_dev_properties);
> + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +
> + /* Host bridges aren't hotpluggable. FIXME: spec reference */
> + dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo pxb_cxl_dev_info = {
> + .name = TYPE_PXB_CXL_DEVICE,
> + .parent = TYPE_PCI_DEVICE,
> + .instance_size = sizeof(PXBDev),
> + .class_init = pxb_cxl_dev_class_init,
> + .interfaces =
> + (InterfaceInfo[]){
> + { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> + {},
> + },
> +};
> +
> static void pxb_register_types(void)
> {
> type_register_static(&pxb_bus_info);
> type_register_static(&pxb_pcie_bus_info);
> + type_register_static(&pxb_cxl_bus_info);
> type_register_static(&pxb_host_info);
> type_register_static(&pxb_dev_info);
> type_register_static(&pxb_pcie_dev_info);
> + type_register_static(&pxb_cxl_dev_info);
> }
>
> type_init(pxb_register_types)
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index db88788c4b..67eed889a4 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -220,6 +220,12 @@ static const TypeInfo pcie_bus_info = {
> .class_init = pcie_bus_class_init,
> };
>
> +static const TypeInfo cxl_bus_info = {
> + .name = TYPE_CXL_BUS,
> + .parent = TYPE_PCIE_BUS,
> + .class_init = pcie_bus_class_init,
> +};
> +
> static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
> static void pci_update_mappings(PCIDevice *d);
> static void pci_irq_handler(void *opaque, int irq_num, int level);
> @@ -2847,6 +2853,7 @@ static void pci_register_types(void)
> {
> type_register_static(&pci_bus_info);
> type_register_static(&pcie_bus_info);
> + type_register_static(&cxl_bus_info);
> type_register_static(&conventional_pci_interface_info);
> type_register_static(&cxl_interface_info);
> type_register_static(&pcie_interface_info);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 4e6fd59fdd..52267ff69e 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -405,6 +405,7 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
> #define TYPE_PCI_BUS "PCI"
> OBJECT_DECLARE_TYPE(PCIBus, PCIBusClass, PCI_BUS)
> #define TYPE_PCIE_BUS "PCIE"
> +#define TYPE_CXL_BUS "CXL"
>
> bool pci_bus_is_express(PCIBus *bus);
>
> @@ -753,6 +754,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev)
> pci_irq_deassert(pci_dev);
> }
>
> +static inline int pci_is_cxl(const PCIDevice *d)
> +{
> + return d->cap_present & QEMU_PCIE_CAP_CXL;
> +}
> +
> static inline int pci_is_express(const PCIDevice *d)
> {
> return d->cap_present & QEMU_PCI_CAP_EXPRESS;
next prev parent reply other threads:[~2020-11-16 16:46 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-11 5:46 [RFC PATCH 00/25] Introduce CXL 2.0 Emulation Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 01/25] Temp: Add the PCI_EXT_ID_DVSEC definition to the qemu pci_regs.h copy Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 02/25] hw/pci/cxl: Add a CXL component type (interface) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 03/25] hw/cxl/component: Introduce CXL components (8.1.x, 8.2.5) Ben Widawsky
2020-11-16 12:03 ` Jonathan Cameron
2020-11-16 19:19 ` Ben Widawsky
2020-11-17 12:29 ` Jonathan Cameron
2020-11-24 23:09 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 04/25] hw/cxl/device: Introduce a CXL device (8.2.8) Ben Widawsky
2020-11-16 13:07 ` Jonathan Cameron
2020-11-16 21:11 ` Ben Widawsky
2020-11-17 14:21 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 05/25] hw/cxl/device: Implement the CAP array (8.2.8.1-2) Ben Widawsky
2020-11-16 13:11 ` Jonathan Cameron
2020-11-16 18:08 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 06/25] hw/cxl/device: Add device status (8.2.8.3) Ben Widawsky
2020-11-16 13:16 ` Jonathan Cameron
2020-11-16 21:18 ` Ben Widawsky
2020-11-17 14:24 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 07/25] hw/cxl/device: Implement basic mailbox (8.2.8.4) Ben Widawsky
2020-11-16 13:46 ` Jonathan Cameron
2020-11-16 21:42 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 08/25] hw/cxl/device: Add memory devices (8.2.8.5) Ben Widawsky
2020-11-16 16:37 ` Jonathan Cameron
2020-11-16 21:45 ` Ben Widawsky
2020-11-17 14:31 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 09/25] hw/pxb: Use a type for realizing expanders Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 10/25] hw/pci/cxl: Create a CXL bus type Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 11/25] hw/pxb: Allow creation of a CXL PXB (host bridge) Ben Widawsky
2020-11-16 16:44 ` Jonathan Cameron [this message]
2020-11-16 22:01 ` Ben Widawsky
2020-11-17 14:33 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 12/25] acpi/pci: Consolidate host bridge setup Ben Widawsky
2020-11-12 17:46 ` Ben Widawsky
2020-11-16 16:45 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 13/25] hw/pci: Plumb _UID through host bridges Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 14/25] hw/cxl/component: Implement host bridge MMIO (8.2.5, table 142) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 15/25] acpi/pxb/cxl: Reserve host bridge MMIO Ben Widawsky
2020-11-16 16:54 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 16/25] hw/pxb/cxl: Add "windows" for host bridges Ben Widawsky
2020-11-13 0:49 ` Ben Widawsky
2020-11-23 19:12 ` Philippe Mathieu-Daudé
2020-11-11 5:47 ` [RFC PATCH 17/25] hw/cxl/rp: Add a root port Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5) Ben Widawsky
2020-11-12 18:37 ` Eric Blake
2020-11-13 7:47 ` Markus Armbruster
2020-11-25 16:53 ` Ben Widawsky
2020-11-26 6:36 ` Markus Armbruster
2020-11-30 17:07 ` Ben Widawsky
2020-12-01 17:06 ` Markus Armbruster
2020-11-11 5:47 ` [RFC PATCH 19/25] hw/cxl/device: Implement MMIO HDM decoding (8.2.5.12) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 20/25] acpi/cxl: Add _OSC implementation (9.14.2) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 21/25] acpi/cxl: Introduce a compat-driver UUID for CXL _OSC Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 22/25] acpi/cxl: Create the CEDT (9.14.1) Ben Widawsky
2020-11-16 17:15 ` Jonathan Cameron
2020-11-16 22:05 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 23/25] Temp: acpi/cxl: Add ACPI0017 (CEDT awareness) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 24/25] WIP: i386/cxl: Initialize a host bridge Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 25/25] qtest/cxl: Add very basic sanity tests Ben Widawsky
2020-11-16 17:21 ` [RFC PATCH 00/25] Introduce CXL 2.0 Emulation Jonathan Cameron
2020-11-16 18:06 ` Ben Widawsky
2020-11-17 14:09 ` Jonathan Cameron
2020-11-25 18:29 ` Ben Widawsky
2020-12-04 14:27 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201116164409.00005f3b@Huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=ben.widawsky@intel.com \
--cc=dan.j.williams@intel.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).