| Message ID | 166993222098.1995348.16604163596374520890.stgit@dwillia2-xfh.jf.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d18bc74aced65e4ad68a30ac8db883398141e918 |
| Headers | show |
| Series | cxl, nvdimm: Move CPU cache management to region drivers | expand |
On 12/1/2022 3:03 PM, Dan Williams wrote: > A "DPA invalidation event" is any scenario where the contents of a DPA > (Device Physical Address) is modified in a way that is incoherent with > CPU caches, or if the HPA (Host Physical Address) to DPA association > changes due to a remapping event. > > PMEM security events like Unlock and Passphrase Secure Erase already > manage caches through LIBNVDIMM, so that leaves HPA to DPA remap events > that need cache management by the CXL core. Those only happen when the > boot time CXL configuration has changed. That event occurs when > userspace attaches an endpoint decoder to a region configuration, and > that region is subsequently activated. > > The implications of not invalidating caches between remap events is that > reads from the region at different points in time may return different > results due to stale cached data from the previous HPA to DPA mapping. > Without a guarantee that the region contents after cxl_region_probe() > are written before being read (a layering-violation assumption that > cxl_region_probe() can not make) the CXL subsystem needs to ensure that > reads that precede writes see consistent results. > > A CONFIG_CXL_REGION_INVALIDATION_TEST option is added to support debug > and unit testing of the CXL implementation in QEMU or other environments > where cpu_cache_has_invalidate_memregion() returns false. This may prove > too restrictive for QEMU where the HDM decoders are emulated, but in > that case the CXL subsystem needs some new mechanism / indication that > the HDM decoder is emulated and not a passthrough of real hardware. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/Kconfig | 18 ++++++++++++++++++ > drivers/cxl/core/region.c | 31 +++++++++++++++++++++++++++++++ > drivers/cxl/cxl.h | 8 ++++++++ > drivers/cxl/security.c | 14 -------------- > 4 files changed, 57 insertions(+), 14 deletions(-) > > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig > index 768ced3d6fe8..0ac53c422c31 100644 > --- a/drivers/cxl/Kconfig > +++ b/drivers/cxl/Kconfig > @@ -111,4 +111,22 @@ config CXL_REGION > select MEMREGION > select GET_FREE_REGION > > +config CXL_REGION_INVALIDATION_TEST > + bool "CXL: Region Cache Management Bypass (TEST)" > + depends on CXL_REGION > + help > + CXL Region management and security operations potentially invalidate > + the content of CPU caches without notifiying those caches to > + invalidate the affected cachelines. The CXL Region driver attempts > + to invalidate caches when those events occur. If that invalidation > + fails the region will fail to enable. Reasons for cache > + invalidation failure are due to the CPU not providing a cache > + invalidation mechanism. For example usage of wbinvd is restricted to > + bare metal x86. However, for testing purposes toggling this option > + can disable that data integrity safety and proceed with enabling > + regions when there might be conflicting contents in the CPU cache. > + > + If unsure, or if this kernel is meant for production environments, > + say N. > + > endif > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 1bc2ebefa2a5..3a6c3f84015f 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -1403,6 +1403,8 @@ static int attach_target(struct cxl_region *cxlr, const char *decoder, int pos) > goto out; > down_read(&cxl_dpa_rwsem); > rc = cxl_region_attach(cxlr, to_cxl_endpoint_decoder(dev), pos); > + if (rc == 0) > + set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > up_read(&cxl_dpa_rwsem); > up_write(&cxl_region_rwsem); > out: > @@ -1900,6 +1902,30 @@ static int devm_cxl_add_pmem_region(struct cxl_region *cxlr) > return rc; > } > > +static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) > +{ > + if (!test_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags)) > + return 0; > + > + if (!cpu_cache_has_invalidate_memregion()) { > + if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)) { > + dev_warn( > + &cxlr->dev, > + "Bypassing cpu_cache_invalidate_memergion() for testing!\n"); > + clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > + return 0; > + } else { > + dev_err(&cxlr->dev, > + "Failed to synchronize CPU cache state\n"); > + return -ENXIO; > + } > + } > + > + cpu_cache_invalidate_memregion(IORES_DESC_CXL); > + clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); > + return 0; > +} > + > static int cxl_region_probe(struct device *dev) > { > struct cxl_region *cxlr = to_cxl_region(dev); > @@ -1915,12 +1941,16 @@ static int cxl_region_probe(struct device *dev) > if (p->state < CXL_CONFIG_COMMIT) { > dev_dbg(&cxlr->dev, "config state: %d\n", p->state); > rc = -ENXIO; > + goto out; > } > > + rc = cxl_region_invalidate_memregion(cxlr); > + > /* > * From this point on any path that changes the region's state away from > * CXL_CONFIG_COMMIT is also responsible for releasing the driver. > */ > +out: > up_read(&cxl_region_rwsem); > > if (rc) > @@ -1953,4 +1983,5 @@ void cxl_region_exit(void) > } > > MODULE_IMPORT_NS(CXL); > +MODULE_IMPORT_NS(DEVMEM); > MODULE_ALIAS_CXL(CXL_DEVICE_REGION); > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index b433e541a054..e5e1abceeca7 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -380,12 +380,19 @@ struct cxl_region_params { > int nr_targets; > }; > > +/* > + * Flag whether this region needs to have its HPA span synchronized with > + * CPU cache state at region activation time. > + */ > +#define CXL_REGION_F_INCOHERENT 0 > + > /** > * struct cxl_region - CXL region > * @dev: This region's device > * @id: This region's id. Id is globally unique across all regions > * @mode: Endpoint decoder allocation / access mode > * @type: Endpoint decoder target type > + * @flags: Region state flags > * @params: active + config params for the region > */ > struct cxl_region { > @@ -393,6 +400,7 @@ struct cxl_region { > int id; > enum cxl_decoder_mode mode; > enum cxl_decoder_type type; > + unsigned long flags; > struct cxl_region_params params; > }; > > diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c > index cbd005ceb091..5484d4eecfd1 100644 > --- a/drivers/cxl/security.c > +++ b/drivers/cxl/security.c > @@ -120,17 +120,12 @@ static int cxl_pmem_security_unlock(struct nvdimm *nvdimm, > u8 pass[NVDIMM_PASSPHRASE_LEN]; > int rc; > > - if (!cpu_cache_has_invalidate_memregion()) > - return -EINVAL; > - > memcpy(pass, key_data->data, NVDIMM_PASSPHRASE_LEN); > rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_UNLOCK, > pass, NVDIMM_PASSPHRASE_LEN, NULL, 0); > if (rc < 0) > return rc; > > - /* DIMM unlocked, invalidate all CPU caches before we read it */ > - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY); > return 0; > } > > @@ -144,21 +139,14 @@ static int cxl_pmem_security_passphrase_erase(struct nvdimm *nvdimm, > struct cxl_pass_erase erase; > int rc; > > - if (!cpu_cache_has_invalidate_memregion()) > - return -EINVAL; > - > erase.type = ptype == NVDIMM_MASTER ? > CXL_PMEM_SEC_PASS_MASTER : CXL_PMEM_SEC_PASS_USER; > memcpy(erase.pass, key->data, NVDIMM_PASSPHRASE_LEN); > - /* Flush all cache before we erase mem device */ > - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY); > rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE, > &erase, sizeof(erase), NULL, 0); > if (rc < 0) > return rc; > > - /* mem device erased, invalidate all CPU caches before data is read */ > - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY); > return 0; > } > > @@ -173,5 +161,3 @@ static const struct nvdimm_security_ops __cxl_security_ops = { > }; > > const struct nvdimm_security_ops *cxl_security_ops = &__cxl_security_ops; > - > -MODULE_IMPORT_NS(DEVMEM); >
On Thu, 01 Dec 2022, Dan Williams wrote: >A "DPA invalidation event" is any scenario where the contents of a DPA >(Device Physical Address) is modified in a way that is incoherent with >CPU caches, or if the HPA (Host Physical Address) to DPA association >changes due to a remapping event. > >PMEM security events like Unlock and Passphrase Secure Erase already >manage caches through LIBNVDIMM, Just to be clear, is this is why you get rid of the explicit flushing for the respective commands in security.c? >so that leaves HPA to DPA remap events >that need cache management by the CXL core. Those only happen when the >boot time CXL configuration has changed. That event occurs when >userspace attaches an endpoint decoder to a region configuration, and >that region is subsequently activated. > >The implications of not invalidating caches between remap events is that >reads from the region at different points in time may return different >results due to stale cached data from the previous HPA to DPA mapping. >Without a guarantee that the region contents after cxl_region_probe() >are written before being read (a layering-violation assumption that >cxl_region_probe() can not make) the CXL subsystem needs to ensure that >reads that precede writes see consistent results. Hmm where does this leave us remaping under arm64 which is doesn't have ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION? Back when we were discussing this it was all related to the security stuff, which under arm it could just be easily discarded as not available feature. Thanks, Davidlohr
[ add linux-arm-kernel@lists.infradead.org ] Background for ARM folks, CXL can dynamically reconfigure the target devices that back a given physical memory region. When that happens the CPU cache can be holding cache data from a previous configuration. The mitigation for that scenario on x86 is wbinvd, ARM does not have an equivalent. The result, dynamic region creation is disabled on ARM. In the near term, most CXL is configured pre-boot, but going forward this restriction is untenable. Davidlohr Bueso wrote: > On Thu, 01 Dec 2022, Dan Williams wrote: > > >A "DPA invalidation event" is any scenario where the contents of a DPA > >(Device Physical Address) is modified in a way that is incoherent with > >CPU caches, or if the HPA (Host Physical Address) to DPA association > >changes due to a remapping event. > > > >PMEM security events like Unlock and Passphrase Secure Erase already > >manage caches through LIBNVDIMM, > > Just to be clear, is this is why you get rid of the explicit flushing > for the respective commands in security.c? Correct, because those commands can only be executed through libnvdimm. > > >so that leaves HPA to DPA remap events > >that need cache management by the CXL core. Those only happen when the > >boot time CXL configuration has changed. That event occurs when > >userspace attaches an endpoint decoder to a region configuration, and > >that region is subsequently activated. > > > >The implications of not invalidating caches between remap events is that > >reads from the region at different points in time may return different > >results due to stale cached data from the previous HPA to DPA mapping. > >Without a guarantee that the region contents after cxl_region_probe() > >are written before being read (a layering-violation assumption that > >cxl_region_probe() can not make) the CXL subsystem needs to ensure that > >reads that precede writes see consistent results. > > Hmm where does this leave us remaping under arm64 which is doesn't have > ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION? > > Back when we were discussing this it was all related to the security stuff, > which under arm it could just be easily discarded as not available feature. I can throw out a few strawman options, but really need help from ARM folks to decide where to go next. 1/ Map and loop cache flushing line by line. It works, but for Terabytes of CXL the cost is 10s of seconds of latency to reconfigure a region. That said, region configuration, outside of test scenarios, is typically a "once per bare metal provisioning" event. 2/ Set a configuration dependency that mandates that all CXL memory be routed through the page allocator where it is guaranteed that the memory will be written (zeroed) before use. This restricts some planned use cases for the "Dynamic Capacity Device" capability. 3/ Work with the CXL consortium to extend the back-invalidate concept for general purpose usage to make devices capable of invalidating caches for a new memory region they joined, and mandate it for ARM. This one has a long lead time and a gap for every device in flight currently.
On Mon, 5 Dec 2022 12:10:22 -0800 Dan Williams <dan.j.williams@intel.com> wrote: > [ add linux-arm-kernel@lists.infradead.org ] > > Background for ARM folks, CXL can dynamically reconfigure the target > devices that back a given physical memory region. When that happens the > CPU cache can be holding cache data from a previous configuration. The > mitigation for that scenario on x86 is wbinvd, ARM does not have an > equivalent. The result, dynamic region creation is disabled on ARM. In > the near term, most CXL is configured pre-boot, but going forward this > restriction is untenable. > > Davidlohr Bueso wrote: > > On Thu, 01 Dec 2022, Dan Williams wrote: > > > > >A "DPA invalidation event" is any scenario where the contents of a DPA > > >(Device Physical Address) is modified in a way that is incoherent with > > >CPU caches, or if the HPA (Host Physical Address) to DPA association > > >changes due to a remapping event. > > > > > >PMEM security events like Unlock and Passphrase Secure Erase already > > >manage caches through LIBNVDIMM, > > > > Just to be clear, is this is why you get rid of the explicit flushing > > for the respective commands in security.c? > > Correct, because those commands can only be executed through libnvdimm. > > > > > >so that leaves HPA to DPA remap events > > >that need cache management by the CXL core. Those only happen when the > > >boot time CXL configuration has changed. That event occurs when > > >userspace attaches an endpoint decoder to a region configuration, and > > >that region is subsequently activated. > > > > > >The implications of not invalidating caches between remap events is that > > >reads from the region at different points in time may return different > > >results due to stale cached data from the previous HPA to DPA mapping. > > >Without a guarantee that the region contents after cxl_region_probe() > > >are written before being read (a layering-violation assumption that > > >cxl_region_probe() can not make) the CXL subsystem needs to ensure that > > >reads that precede writes see consistent results. > > > > Hmm where does this leave us remaping under arm64 which is doesn't have > > ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION? > > > > Back when we were discussing this it was all related to the security stuff, > > which under arm it could just be easily discarded as not available feature. > > I can throw out a few strawman options, but really need help from ARM > folks to decide where to go next. +Cc bunch of relevant people. There are discussions underway but I'm not sure anyone will want to give more details here yet. > > 1/ Map and loop cache flushing line by line. It works, but for Terabytes > of CXL the cost is 10s of seconds of latency to reconfigure a region. > That said, region configuration, outside of test scenarios, is typically > a "once per bare metal provisioning" event. > > 2/ Set a configuration dependency that mandates that all CXL memory be > routed through the page allocator where it is guaranteed that the memory > will be written (zeroed) before use. This restricts some planned use > cases for the "Dynamic Capacity Device" capability. This is the only case that's really a problem (to my mind) I hope we will have a more general solution before there is much hardware out there, particularly where sharing is involved. > > 3/ Work with the CXL consortium to extend the back-invalidate concept > for general purpose usage to make devices capable of invalidating caches > for a new memory region they joined, and mandate it for ARM. This one > has a long lead time and a gap for every device in flight currently. There are significant disadvantages in doing this that I suspect will mean this never happens for some classes of device, or is turned off for performance reasons. For anyone curious, go look at the protocol requirements of back invalidate in the CXL 3.0 spec. Jonathan
Hi guys, On 06/12/2022 09:47, Jonathan Cameron wrote: > On Mon, 5 Dec 2022 12:10:22 -0800 > Dan Williams <dan.j.williams@intel.com> wrote: > >> [ add linux-arm-kernel@lists.infradead.org ] >> >> Background for ARM folks, CXL can dynamically reconfigure the target >> devices that back a given physical memory region. When that happens the >> CPU cache can be holding cache data from a previous configuration. The >> mitigation for that scenario on x86 is wbinvd, ARM does not have an >> equivalent. The result, dynamic region creation is disabled on ARM. In >> the near term, most CXL is configured pre-boot, but going forward this >> restriction is untenable. >> >> Davidlohr Bueso wrote: >>> On Thu, 01 Dec 2022, Dan Williams wrote: >>> >>>> A "DPA invalidation event" is any scenario where the contents of a DPA >>>> (Device Physical Address) is modified in a way that is incoherent with >>>> CPU caches, or if the HPA (Host Physical Address) to DPA association >>>> changes due to a remapping event. >>>> >>>> PMEM security events like Unlock and Passphrase Secure Erase already >>>> manage caches through LIBNVDIMM, >>> >>> Just to be clear, is this is why you get rid of the explicit flushing >>> for the respective commands in security.c? >> >> Correct, because those commands can only be executed through libnvdimm. >> >>> >>>> so that leaves HPA to DPA remap events >>>> that need cache management by the CXL core. Those only happen when the >>>> boot time CXL configuration has changed. That event occurs when >>>> userspace attaches an endpoint decoder to a region configuration, and >>>> that region is subsequently activated. >>>> >>>> The implications of not invalidating caches between remap events is that >>>> reads from the region at different points in time may return different >>>> results due to stale cached data from the previous HPA to DPA mapping. >>>> Without a guarantee that the region contents after cxl_region_probe() >>>> are written before being read (a layering-violation assumption that >>>> cxl_region_probe() can not make) the CXL subsystem needs to ensure that >>>> reads that precede writes see consistent results. >>> >>> Hmm where does this leave us remaping under arm64 which is doesn't have >>> ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION? For those reading along at home, ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION is wbinvd. https://lore.kernel.org/linux-cxl/20220919110605.3696-1-dave@stgolabs.net/ We don't have an instruction for arm64 that 'invalidates all caches'. >>> Back when we were discussing this it was all related to the security stuff, >>> which under arm it could just be easily discarded as not available feature. >> >> I can throw out a few strawman options, but really need help from ARM >> folks to decide where to go next. > +Cc bunch of relevant people. There are discussions underway but I'm not sure > anyone will want to give more details here yet. The best we can do today is to use the by-VA invalidate operations in the kernel. This isn't guaranteed to invalidate 'invisible' system caches, which means its not enough for a one-size-fits-all kernel interface. For the NVDIMM secure-erase users of this thing, if there were a system-cache between the CPUs and the NVDIMM, there is nothing the kernel can do to invalidate it. If its CXL specific this would be okay for testing in Qemu, but performance would scale with the size of the region, which would hurt in real world cases. The plan is to add a firmware call so firmware can do things that don't scale with the size of the mapping, and do something platform-specific to the 'invisible' system cache, if there is one. Ideally we wait for the PSCI spec update that describes the firmware call, and make support dependent on that. It looks like the timeline will be March-ish, but there should be an alpha of the spec available much sooner. >> 1/ Map and loop cache flushing line by line. It works, but for Terabytes >> of CXL the cost is 10s of seconds of latency to reconfigure a region. >> That said, region configuration, outside of test scenarios, is typically >> a "once per bare metal provisioning" event. It works for CXL because you'd never have a system-cache in front of the CXL window. Those things don't necessarily receive cache-maintenance because they are supposed to be invisible. D7.4.11 of DDI0487I.a "System level caches" has this horror: | System caches which lie beyond the point of coherency and so are invisible to the | software. The management of such caches is outside the scope of the architecture. (The PoP stuff reaches beyond the PoC, but there isn't a DC CIVAP instruction) Detecting which regions we can't do this for is problematic. >> 2/ Set a configuration dependency that mandates that all CXL memory be >> routed through the page allocator where it is guaranteed that the memory >> will be written (zeroed) before use. This restricts some planned use >> cases for the "Dynamic Capacity Device" capability. > This is the only case that's really a problem (to my mind) I hope we will have > a more general solution before there is much hardware out there, particularly > where sharing is involved. Thanks, James >> 3/ Work with the CXL consortium to extend the back-invalidate concept >> for general purpose usage to make devices capable of invalidating caches >> for a new memory region they joined, and mandate it for ARM. This one >> has a long lead time and a gap for every device in flight currently. > > There are significant disadvantages in doing this that I suspect will mean > this never happens for some classes of device, or is turned off for performance > reasons. For anyone curious, go look at the protocol requirements of back > invalidate in the CXL 3.0 spec. > > Jonathan
diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig index 768ced3d6fe8..0ac53c422c31 100644 --- a/drivers/cxl/Kconfig +++ b/drivers/cxl/Kconfig @@ -111,4 +111,22 @@ config CXL_REGION select MEMREGION select GET_FREE_REGION +config CXL_REGION_INVALIDATION_TEST + bool "CXL: Region Cache Management Bypass (TEST)" + depends on CXL_REGION + help + CXL Region management and security operations potentially invalidate + the content of CPU caches without notifiying those caches to + invalidate the affected cachelines. The CXL Region driver attempts + to invalidate caches when those events occur. If that invalidation + fails the region will fail to enable. Reasons for cache + invalidation failure are due to the CPU not providing a cache + invalidation mechanism. For example usage of wbinvd is restricted to + bare metal x86. However, for testing purposes toggling this option + can disable that data integrity safety and proceed with enabling + regions when there might be conflicting contents in the CPU cache. + + If unsure, or if this kernel is meant for production environments, + say N. + endif diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 1bc2ebefa2a5..3a6c3f84015f 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1403,6 +1403,8 @@ static int attach_target(struct cxl_region *cxlr, const char *decoder, int pos) goto out; down_read(&cxl_dpa_rwsem); rc = cxl_region_attach(cxlr, to_cxl_endpoint_decoder(dev), pos); + if (rc == 0) + set_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); up_read(&cxl_dpa_rwsem); up_write(&cxl_region_rwsem); out: @@ -1900,6 +1902,30 @@ static int devm_cxl_add_pmem_region(struct cxl_region *cxlr) return rc; } +static int cxl_region_invalidate_memregion(struct cxl_region *cxlr) +{ + if (!test_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags)) + return 0; + + if (!cpu_cache_has_invalidate_memregion()) { + if (IS_ENABLED(CONFIG_CXL_REGION_INVALIDATION_TEST)) { + dev_warn( + &cxlr->dev, + "Bypassing cpu_cache_invalidate_memergion() for testing!\n"); + clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); + return 0; + } else { + dev_err(&cxlr->dev, + "Failed to synchronize CPU cache state\n"); + return -ENXIO; + } + } + + cpu_cache_invalidate_memregion(IORES_DESC_CXL); + clear_bit(CXL_REGION_F_INCOHERENT, &cxlr->flags); + return 0; +} + static int cxl_region_probe(struct device *dev) { struct cxl_region *cxlr = to_cxl_region(dev); @@ -1915,12 +1941,16 @@ static int cxl_region_probe(struct device *dev) if (p->state < CXL_CONFIG_COMMIT) { dev_dbg(&cxlr->dev, "config state: %d\n", p->state); rc = -ENXIO; + goto out; } + rc = cxl_region_invalidate_memregion(cxlr); + /* * From this point on any path that changes the region's state away from * CXL_CONFIG_COMMIT is also responsible for releasing the driver. */ +out: up_read(&cxl_region_rwsem); if (rc) @@ -1953,4 +1983,5 @@ void cxl_region_exit(void) } MODULE_IMPORT_NS(CXL); +MODULE_IMPORT_NS(DEVMEM); MODULE_ALIAS_CXL(CXL_DEVICE_REGION); diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index b433e541a054..e5e1abceeca7 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -380,12 +380,19 @@ struct cxl_region_params { int nr_targets; }; +/* + * Flag whether this region needs to have its HPA span synchronized with + * CPU cache state at region activation time. + */ +#define CXL_REGION_F_INCOHERENT 0 + /** * struct cxl_region - CXL region * @dev: This region's device * @id: This region's id. Id is globally unique across all regions * @mode: Endpoint decoder allocation / access mode * @type: Endpoint decoder target type + * @flags: Region state flags * @params: active + config params for the region */ struct cxl_region { @@ -393,6 +400,7 @@ struct cxl_region { int id; enum cxl_decoder_mode mode; enum cxl_decoder_type type; + unsigned long flags; struct cxl_region_params params; }; diff --git a/drivers/cxl/security.c b/drivers/cxl/security.c index cbd005ceb091..5484d4eecfd1 100644 --- a/drivers/cxl/security.c +++ b/drivers/cxl/security.c @@ -120,17 +120,12 @@ static int cxl_pmem_security_unlock(struct nvdimm *nvdimm, u8 pass[NVDIMM_PASSPHRASE_LEN]; int rc; - if (!cpu_cache_has_invalidate_memregion()) - return -EINVAL; - memcpy(pass, key_data->data, NVDIMM_PASSPHRASE_LEN); rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_UNLOCK, pass, NVDIMM_PASSPHRASE_LEN, NULL, 0); if (rc < 0) return rc; - /* DIMM unlocked, invalidate all CPU caches before we read it */ - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY); return 0; } @@ -144,21 +139,14 @@ static int cxl_pmem_security_passphrase_erase(struct nvdimm *nvdimm, struct cxl_pass_erase erase; int rc; - if (!cpu_cache_has_invalidate_memregion()) - return -EINVAL; - erase.type = ptype == NVDIMM_MASTER ? CXL_PMEM_SEC_PASS_MASTER : CXL_PMEM_SEC_PASS_USER; memcpy(erase.pass, key->data, NVDIMM_PASSPHRASE_LEN); - /* Flush all cache before we erase mem device */ - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY); rc = cxl_mbox_send_cmd(cxlds, CXL_MBOX_OP_PASSPHRASE_SECURE_ERASE, &erase, sizeof(erase), NULL, 0); if (rc < 0) return rc; - /* mem device erased, invalidate all CPU caches before data is read */ - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY); return 0; } @@ -173,5 +161,3 @@ static const struct nvdimm_security_ops __cxl_security_ops = { }; const struct nvdimm_security_ops *cxl_security_ops = &__cxl_security_ops; - -MODULE_IMPORT_NS(DEVMEM);
A "DPA invalidation event" is any scenario where the contents of a DPA (Device Physical Address) is modified in a way that is incoherent with CPU caches, or if the HPA (Host Physical Address) to DPA association changes due to a remapping event. PMEM security events like Unlock and Passphrase Secure Erase already manage caches through LIBNVDIMM, so that leaves HPA to DPA remap events that need cache management by the CXL core. Those only happen when the boot time CXL configuration has changed. That event occurs when userspace attaches an endpoint decoder to a region configuration, and that region is subsequently activated. The implications of not invalidating caches between remap events is that reads from the region at different points in time may return different results due to stale cached data from the previous HPA to DPA mapping. Without a guarantee that the region contents after cxl_region_probe() are written before being read (a layering-violation assumption that cxl_region_probe() can not make) the CXL subsystem needs to ensure that reads that precede writes see consistent results. A CONFIG_CXL_REGION_INVALIDATION_TEST option is added to support debug and unit testing of the CXL implementation in QEMU or other environments where cpu_cache_has_invalidate_memregion() returns false. This may prove too restrictive for QEMU where the HDM decoders are emulated, but in that case the CXL subsystem needs some new mechanism / indication that the HDM decoder is emulated and not a passthrough of real hardware. Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/Kconfig | 18 ++++++++++++++++++ drivers/cxl/core/region.c | 31 +++++++++++++++++++++++++++++++ drivers/cxl/cxl.h | 8 ++++++++ drivers/cxl/security.c | 14 -------------- 4 files changed, 57 insertions(+), 14 deletions(-)