From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: "Peter Maydell" <peter.maydell@linaro.org>, "Shreyas Shah" <shreyas.shah@elastics.cloud>, "Ben Widawsky" <ben.widawsky@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Marcel Apfelbaum" <marcel@redhat.com>, "Samarth Saxena" <samarths@cadence.com>, "Chris Browy" <cbrowy@avery-design.com>, "Markus Armbruster" <armbru@redhat.com>, "Peter Xu" <peterx@redhat.com>, qemu-devel@nongnu.org, linuxarm@huawei.com, linux-cxl@vger.kernel.org, "Igor Mammedov" <imammedo@redhat.com>, "Saransh Gupta1" <saransh@ibm.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Dan Williams" <dan.j.williams@intel.com>, "David Hildenbrand" <david@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Shameerali Kolothum Thodi" <shameerali.kolothum.thodi@huawei.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org> Subject: Re: [PATCH v7 00/46] CXl 2.0 emulation Support Date: Fri, 18 Mar 2022 10:08:46 +0000 [thread overview] Message-ID: <20220318100846.000033a8@Huawei.com> (raw) In-Reply-To: <d710c6e1-a9b7-ed0d-ca23-c4315355268c@ilande.co.uk> On Fri, 18 Mar 2022 08:14:58 +0000 Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > On 17/03/2022 16:47, Jonathan Cameron via wrote: > > >> Ah great! As you've already noticed my particular case was performing partial > >> decoding on a memory region, but there are no issues if you need to dispatch to > >> another existing address space such as PCI/IOMMU. Creating a separate address space > >> per device shouldn't be an issue either, as that's effectively how the PCI bus master > >> requests are handled. > >> > >> The address spaces are visible in "info mtree" so if you haven't already, I would > >> recommend generating a dynamic name for the address space based upon the device > >> name/address to make it easier for development and debugging. > > info mtree already provides the following with a static name > > address-space: cxl-type3-dpa-space > > 0000000000000000-000000000fffffff (prio 0, nv-ram): cxl-mem2 > > > > So the device association is there anyway. Hence I'm not sure a dynamic name adds > > a lot on this occasion and code is simpler without making it dynamic. > > Is this using a single address space for multiple memory devices, or one per device > as you were suggesting in the thread? If it is one per device and cxl-mem2 is the > value of the -device id parameter, I still think it is worth adding the same device > id into the address space name for the sake of a g_strdup_printf() and corresponding > g_free(). One per device. Ultimately when I add volatile memory support we'll end up with possibly having to add an mr as a container for the two hostmem mr. Looking again, the name above is actually the id of the mr, not the type3 device. Probably better to optionally use the type3 device name if available. I'll make the name something like cxl-type3-dpa-space-cxl-pmem3 if id available and fall back to cxl-type3-dpa-space as before if not. > > Alas I don't currently have the time (and enough knowledge of CXL!) to do a more > comprehensive review of the patches, but a quick skim of the series suggests it seems > quite mature. The only thing that I noticed was that there doesn't seem to be any > trace-events added, which I think may be useful to aid driver developers if they need > to debug some of the memory access routing. Good suggestion. I'm inclined to add them in a follow up patch though because this patch set is already somewhat unmanageable from point of view of review. I already have a number of other patches queued up for a second series adding more functionality. > > Finally I should point out that there are a number of more experienced PCI developers > on the CC list than me, and they should have the final say on patch review. So please > consider these comments as recommendations based upon my development work on QEMU, > and not as a NAK for proceeding with the series :) No problem and thanks for your help as (I think) you've solved the biggest open issue :) Jonathan > > > ATB, > > Mark.
WARNING: multiple messages have this Message-ID (diff)
From: Jonathan Cameron via <qemu-devel@nongnu.org> To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: "Peter Maydell" <peter.maydell@linaro.org>, "Shreyas Shah" <shreyas.shah@elastics.cloud>, "Ben Widawsky" <ben.widawsky@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Marcel Apfelbaum" <marcel@redhat.com>, "Samarth Saxena" <samarths@cadence.com>, "Chris Browy" <cbrowy@avery-design.com>, "Markus Armbruster" <armbru@redhat.com>, "Peter Xu" <peterx@redhat.com>, qemu-devel@nongnu.org, linuxarm@huawei.com, linux-cxl@vger.kernel.org, "Igor Mammedov" <imammedo@redhat.com>, "Saransh Gupta1" <saransh@ibm.com>, "Paolo Bonzini" <pbonzini@redhat.com>, "Dan Williams" <dan.j.williams@intel.com>, "David Hildenbrand" <david@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Shameerali Kolothum Thodi" <shameerali.kolothum.thodi@huawei.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org> Subject: Re: [PATCH v7 00/46] CXl 2.0 emulation Support Date: Fri, 18 Mar 2022 10:08:46 +0000 [thread overview] Message-ID: <20220318100846.000033a8@Huawei.com> (raw) In-Reply-To: <d710c6e1-a9b7-ed0d-ca23-c4315355268c@ilande.co.uk> On Fri, 18 Mar 2022 08:14:58 +0000 Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > On 17/03/2022 16:47, Jonathan Cameron via wrote: > > >> Ah great! As you've already noticed my particular case was performing partial > >> decoding on a memory region, but there are no issues if you need to dispatch to > >> another existing address space such as PCI/IOMMU. Creating a separate address space > >> per device shouldn't be an issue either, as that's effectively how the PCI bus master > >> requests are handled. > >> > >> The address spaces are visible in "info mtree" so if you haven't already, I would > >> recommend generating a dynamic name for the address space based upon the device > >> name/address to make it easier for development and debugging. > > info mtree already provides the following with a static name > > address-space: cxl-type3-dpa-space > > 0000000000000000-000000000fffffff (prio 0, nv-ram): cxl-mem2 > > > > So the device association is there anyway. Hence I'm not sure a dynamic name adds > > a lot on this occasion and code is simpler without making it dynamic. > > Is this using a single address space for multiple memory devices, or one per device > as you were suggesting in the thread? If it is one per device and cxl-mem2 is the > value of the -device id parameter, I still think it is worth adding the same device > id into the address space name for the sake of a g_strdup_printf() and corresponding > g_free(). One per device. Ultimately when I add volatile memory support we'll end up with possibly having to add an mr as a container for the two hostmem mr. Looking again, the name above is actually the id of the mr, not the type3 device. Probably better to optionally use the type3 device name if available. I'll make the name something like cxl-type3-dpa-space-cxl-pmem3 if id available and fall back to cxl-type3-dpa-space as before if not. > > Alas I don't currently have the time (and enough knowledge of CXL!) to do a more > comprehensive review of the patches, but a quick skim of the series suggests it seems > quite mature. The only thing that I noticed was that there doesn't seem to be any > trace-events added, which I think may be useful to aid driver developers if they need > to debug some of the memory access routing. Good suggestion. I'm inclined to add them in a follow up patch though because this patch set is already somewhat unmanageable from point of view of review. I already have a number of other patches queued up for a second series adding more functionality. > > Finally I should point out that there are a number of more experienced PCI developers > on the CC list than me, and they should have the final say on patch review. So please > consider these comments as recommendations based upon my development work on QEMU, > and not as a NAK for proceeding with the series :) No problem and thanks for your help as (I think) you've solved the biggest open issue :) Jonathan > > > ATB, > > Mark.
next prev parent reply other threads:[~2022-03-18 10:08 UTC|newest] Thread overview: 124+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-06 17:40 [PATCH v7 00/46] CXl 2.0 emulation Support Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 01/46] hw/pci/cxl: Add a CXL component type (interface) Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 02/46] hw/cxl/component: Introduce CXL components (8.1.x, 8.2.5) Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 03/46] MAINTAINERS: Add entry for Compute Express Link Emulation Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 04/46] hw/cxl/device: Introduce a CXL device (8.2.8) Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 05/46] hw/cxl/device: Implement the CAP array (8.2.8.1-2) Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 06/46] hw/cxl/device: Implement basic mailbox (8.2.8.4) Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 07/46] hw/cxl/device: Add memory device utilities Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:40 ` [PATCH v7 08/46] hw/cxl/device: Add cheap EVENTS implementation (8.2.9.1) Jonathan Cameron 2022-03-06 17:40 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 09/46] hw/cxl/device: Timestamp implementation (8.2.9.3) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 10/46] hw/cxl/device: Add log commands (8.2.9.4) + CEL Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 11/46] hw/pxb: Use a type for realizing expanders Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 12/46] hw/pci/cxl: Create a CXL bus type Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 13/46] cxl: Machine level control on whether CXL support is enabled Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 14/46] hw/pxb: Allow creation of a CXL PXB (host bridge) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 15/46] qtest/cxl: Introduce initial test for pxb-cxl only Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 16/46] hw/cxl/rp: Add a root port Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 17/46] hw/cxl/device: Add a memory device (8.2.8.5) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 18/46] hw/cxl/device: Implement MMIO HDM decoding (8.2.5.12) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 19/46] hw/cxl/device: Add some trivial commands Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 20/46] hw/cxl/device: Plumb real Label Storage Area (LSA) sizing Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 21/46] hw/cxl/device: Implement get/set Label Storage Area (LSA) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 22/46] qtests/cxl: Add initial root port and CXL type3 tests Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 23/46] hw/cxl/component: Implement host bridge MMIO (8.2.5, table 142) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 24/46] acpi/cxl: Add _OSC implementation (9.14.2) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 21:31 ` Michael S. Tsirkin 2022-03-06 21:31 ` Michael S. Tsirkin 2022-03-07 17:01 ` Jonathan Cameron 2022-03-07 17:01 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 25/46] acpi/cxl: Create the CEDT (9.14.1) Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 26/46] hw/cxl/component: Add utils for interleave parameter encoding/decoding Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 27/46] hw/cxl/host: Add support for CXL Fixed Memory Windows Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 28/46] acpi/cxl: Introduce CFMWS structures in CEDT Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 29/46] hw/pci-host/gpex-acpi: Add support for dsdt construction for pxb-cxl Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 30/46] pci/pcie_port: Add pci_find_port_by_pn() Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 31/46] CXL/cxl_component: Add cxl_get_hb_cstate() Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 32/46] mem/cxl_type3: Add read and write functions for associated hostmem Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 33/46] cxl/cxl-host: Add memops for CFMWS region Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 34/46] RFC: softmmu/memory: Add ops to memory_region_ram_init_from_file Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 35/46] hw/cxl/component Add a dumb HDM decoder handler Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 36/46] i386/pc: Enable CXL fixed memory windows Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 37/46] tests/acpi: q35: Allow addition of a CXL test Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 38/46] qtests/bios-tables-test: Add a test for CXL emulation Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 39/46] tests/acpi: Add tables " Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 40/46] qtest/cxl: Add more complex test cases with CFMWs Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 41/46] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 42/46] qtest/cxl: Add aarch64 virt test for CXL Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 43/46] docs/cxl: Add initial Compute eXpress Link (CXL) documentation Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 44/46] pci-bridge/cxl_upstream: Add a CXL switch upstream port Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 45/46] pci-bridge/cxl_downstream: Add a CXL switch downstream port Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 17:41 ` [PATCH v7 46/46] cxl/cxl-host: Support interleave decoding with one level of switches Jonathan Cameron 2022-03-06 17:41 ` Jonathan Cameron via 2022-03-06 21:33 ` [PATCH v7 00/46] CXl 2.0 emulation Support Michael S. Tsirkin 2022-03-06 21:33 ` Michael S. Tsirkin 2022-03-07 9:39 ` Jonathan Cameron via 2022-03-07 9:39 ` Jonathan Cameron 2022-03-09 8:15 ` Peter Xu 2022-03-09 8:15 ` Peter Xu 2022-03-09 11:28 ` Jonathan Cameron 2022-03-09 11:28 ` Jonathan Cameron via 2022-03-10 8:02 ` Peter Xu 2022-03-10 8:02 ` Peter Xu 2022-03-16 16:50 ` Jonathan Cameron 2022-03-16 16:50 ` Jonathan Cameron via 2022-03-16 17:16 ` Mark Cave-Ayland 2022-03-16 17:16 ` Mark Cave-Ayland 2022-03-16 17:58 ` Jonathan Cameron 2022-03-16 17:58 ` Jonathan Cameron via 2022-03-16 18:26 ` Jonathan Cameron 2022-03-16 18:26 ` Jonathan Cameron via 2022-03-17 8:12 ` Mark Cave-Ayland 2022-03-17 8:12 ` Mark Cave-Ayland 2022-03-17 16:47 ` Jonathan Cameron 2022-03-17 16:47 ` Jonathan Cameron via 2022-03-18 8:14 ` Mark Cave-Ayland 2022-03-18 8:14 ` Mark Cave-Ayland 2022-03-18 10:08 ` Jonathan Cameron [this message] 2022-03-18 10:08 ` Jonathan Cameron via
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=20220318100846.000033a8@Huawei.com \ --to=jonathan.cameron@huawei.com \ --cc=alex.bennee@linaro.org \ --cc=armbru@redhat.com \ --cc=ben.widawsky@intel.com \ --cc=cbrowy@avery-design.com \ --cc=dan.j.williams@intel.com \ --cc=david@redhat.com \ --cc=f4bug@amsat.org \ --cc=imammedo@redhat.com \ --cc=linux-cxl@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=marcel@redhat.com \ --cc=mark.cave-ayland@ilande.co.uk \ --cc=mst@redhat.com \ --cc=pbonzini@redhat.com \ --cc=peter.maydell@linaro.org \ --cc=peterx@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=samarths@cadence.com \ --cc=saransh@ibm.com \ --cc=shameerali.kolothum.thodi@huawei.com \ --cc=shreyas.shah@elastics.cloud \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.