Re: [PATCH v10 27/45] hw/cxl/host: Add support for CXL Fixed Memory Windows.

From: Paolo Bonzini <pbonzini@redhat.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	linuxarm@huawei.com, qemu-devel@nongnu.org,
	alex.bennee@linaro.org, Marcel Apfelbaum <marcel@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	Adam Manzanares <a.manzanares@samsung.com>,
	Tong Zhang <ztong0001@gmail.com>
Cc: linux-cxl@vger.kernel.org, Ben Widawsky <ben.widawsky@intel.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Shameerali Kolothum Thodi  <shameerali.kolothum.thodi@huawei.com>,
	f4bug@amsat.org, Peter Xu <peterx@redhat.com>,
	David Hildenbrand <david@redhat.com>,
	Saransh Gupta1 <saransh@ibm.com>,
	Shreyas Shah <shreyas.shah@elastics.cloud>,
	Chris Browy <cbrowy@avery-design.com>,
	Samarth Saxena <samarths@cadence.com>,
	Dan Williams <dan.j.williams@intel.com>,
	k.jensen@samsung.com, dave@stgolabs.net,
	Alison Schofield <alison.schofield@intel.com>
Subject: Re: [PATCH v10 27/45] hw/cxl/host: Add support for CXL Fixed Memory Windows.
Date: Sat, 21 May 2022 11:07:13 +0200	[thread overview]
Message-ID: <0c0055ac-8d94-37d9-62c2-58c3da5461d3@redhat.com> (raw)
In-Reply-To: <20220429144110.25167-28-Jonathan.Cameron@huawei.com>

On 4/29/22 16:40, Jonathan Cameron via wrote:
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> 
> The concept of these is introduced in [1] in terms of the
> description the CEDT ACPI table. The principal is more general.
> Unlike once traffic hits the CXL root bridges, the host system
> memory address routing is implementation defined and effectively
> static once observable by standard / generic system software.
> Each CXL Fixed Memory Windows (CFMW) is a region of PA space
> which has fixed system dependent routing configured so that
> accesses can be routed to the CXL devices below a set of target
> root bridges. The accesses may be interleaved across multiple
> root bridges.
> 
> For QEMU we could have fully specified these regions in terms
> of a base PA + size, but as the absolute address does not matter
> it is simpler to let individual platforms place the memory regions.
> 
> ExampleS:
> -cxl-fixed-memory-window targets.0=cxl.0,size=128G
> -cxl-fixed-memory-window targets.0=cxl.1,size=128G
> -cxl-fixed-memory-window targets.0=cxl0,targets.1=cxl.1,size=256G,interleave-granularity=2k

Sorry for the late review, but: no more command line options should
be added to QEMU.  This should be:

-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.size=128G \
-M cxl-fmw.1.targets.0=cxl.1,cxl-fmw.1.size=128G \
-M cxl-fmw.2.targets.0=cxl.0,cxl-fmw.2.targets.1=cxl.1,cxl-fmw.2.size=128G,cxl-fmw.2.interleave-granularity=2k

I'm not against adding a shortcut as above, but the implementation should
be entirely in MachineState using a QOM property.

The reason for this is that we're looking into doing machine setup entirely
via RPC, and any extra option is a new command to be implemented.

Would you be able to do the change?  Since you are already using QAPI to
deserialize the option it should not be hard.  I can promise a quick review,
and I can also provide with an example conversion for -boot at
https://lore.kernel.org/r/20220414165300.555321-3-pbonzini@redhat.com/.

Paolo