> -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > Sent: Tuesday, April 20, 2021 4:04 AM > To: Dan Williams <dan.j.williams@xxxxxxxxx> > Cc: Vikram Sethi <vsethi@xxxxxxxxxx>; linux-cxl@xxxxxxxxxxxxxxx; linux- > acpi@xxxxxxxxxxxxxxx; Natu, Mahesh <mahesh.natu@xxxxxxxxx>; Douglas, Chet R > <chet.r.douglas@xxxxxxxxx>; Verma, Vishal L <vishal.l.verma@xxxxxxxxx>; > Widawsky, Ben <ben.widawsky@xxxxxxxxx>; Samer El-Haj-Mahmoud <Samer.El- > Haj-Mahmoud@xxxxxxx>; Thanu Rangarajan <Thanu.Rangarajan@xxxxxxx> > Subject: Re: [ACPI Code First ECN v2]: Generic Port, performace data for hotplug > memory > > External email: Use caution opening links or attachments > > > On Mon, 19 Apr 2021 22:08:39 -0700 > Dan Williams <dan.j.williams@xxxxxxxxx> wrote: > > > On Mon, Apr 19, 2021 at 9:22 PM Vikram Sethi <vsethi@xxxxxxxxxx> wrote: > > > > > > > From: Dan Williams <dan.j.williams@xxxxxxxxx> > > > > On Mon, Apr 19, 2021 at 3:56 PM Vikram Sethi <vsethi@xxxxxxxxxx> wrote: > > > > [..] > > > > > > * Replace all instances of "Initiator" with "Initiator / Port" in "Table > > > > > > 5.59 Flags - Generic Initiator Affinity Structure", including the > > > > > > table name. > > > > > > > > > > I wanted to discuss the implications of a CXL host bridge implementation > that > > > > > does not set the "Architectural Transactions" bit/flag in the aforementioned > > > > > Flags in Table 5.59. > > > > > > > > > > Since the kernel would be expecting all "System RAM" to have equivalent > > > > > Functional properties, if HDM cannot have all the same functionality, then > in > > > > > the absence of ISA specific ACPI tables clarifying what architectural feature > isn't > > > > > supported, the kernel may be forced to not online the HDM memory as > system > > > > > RAM. If there is more granular expression of what features are lacking in a > ISA > > > > > Specific table (eg Memory Tagging/Memory Protection keys not > supported), > > > > > the kernel could choose to not enable that feature in all of system RAM (if > > > > > discrepancy discovered at boot), but still online the HDM as System RAM. > > > > > > > > > > To that end, it may be useful to clarify this to host vendors by way of an > > > > > Implementation note (ideally in the CXL specification). Something like: > > > > > "CXL hosts are encouraged to support all architectural features in HDM > > > > > as they do in CPU attached memory to avoid either the memory from > > > > > being onlined as System RAM, or the architectural feature being disabled. > > > > > Hosts must indicate architectural parity for HDM in ACPI SRAT > > > > > “Generic Port” flags “Architectural transactions” bit by setting it to 1. > > > > > A port that sets this bit to 0 will need ISA specific ways/ACPI tables to > > > > > describe which specific ISA features would not work in HDM, so an OS > > > > > could disable that memory or that feature." > > > > > > > > > > Thoughts? > > > > > > > > The problem, as you know, is that those features are already defined > > > > without those "ISA specific ways / ACPI tables". I think it simply > > > > must be the case that the only agent in the system that is aware of > > > > the intersection of capabilities between ISA and CXL (platform > > > > firmware) must mitigate the conflict. I.e. so that the CXL > > > > infrastructure need not worry about ISA feature capability and vice > > > > versa. To me, this looks like a platform firmware pre-boot > > > > configuration menu / switch that turns off CXL (declines to publish > > > > ACPI0016 devices) if incompatible ISA feature "X" is enabled, or the > > > > reverse turns off ISA feature "X" if CXL is enabled. In other words, > > > > the conflict needs to be resolved before the OS boots, setting this > > > > bit to 0 is not a viable option for mitigating the conflict because > > > > there is no requirement for the OS to even look at this flag. > > > > > > Leaving it to Firmware is easier for the OS, but could be a couple > > > of issues with that: > > > Platform firmware may not have a way of disabling ISA feature > > > if it is directly visible to the OS via CPU ID registers, and the > > > registers can't be trapped to some FW and values adjusted > > > on access > > > Platform Firmware may not know if the OS supports a specific > > > Feature (code may not exist or not default option etc) so it > > > may be premature/suboptimal to disable CXL hostbridge > > > altogether. Although I suppose a UEFI variable type knob > > > could be adjusted in this case and take effect on reboot. > > > > > > Also, for some *future* ISA features where it may be possible and > > > practical to define ISA feature support discovery per NUMA > > > node/address range w/ ACPI (prior to userspace ABI being set), > > > the platform would want to enable the CXL host bridge and leave > > > selective enablement of the feature to the OS. Yes, this is messy > > > and best avoided, but it may make sense depending on ISA > > > feature and how messy it makes user space. I'm personally > > > not in favor of this latter option, but I'm told this was discussed > > > in other Coherent interconnect forums and chosen as a path > > > forward. > > > > I think it's reasonable for new stuff to define _OSC or other opt-in > > requirements to allow the OS to manage ISA vs CXL conflict policy. For > > existing conflicts the only reliable mechanism is decline to publish > > ACPI0016 if platform firmware can enumerate an ISA feature that it is > > not supported on CXL. So I think the proposal here is a recommendation > > for platform firmware implementations that they are responsible for > > this conflict resolution unless / until other mechanisms arrive. > > Agreed with one addition. It should be possible to retrofit negotiation > for existing features as well. Default policy should be that it's firmware's > problem but if the OS uses _OSC to negotiate something else then it may > be possible to be more flexible. As long as the default is safe, relaxing > that can happen once mechanisms are defined. The actual decision on > whether to enable ACPI0016 can for example be pushed into AML code. > Relaxations could happen only when ABI to userspace isn't already set and in use though, no? What about Coherent device memory in type 2 devices? That is not EFI conventional memory but once some part has been onlined by a driver by calling something like add_memory_driver_managed, that part should have all the properties of System RAM as well. Not clear that Platform firmware would know if a driver intents to online some or all of that memory.
