On Tue, 30 Sep 2025, Jonathan Cameron wrote:
On Mon, 29 Sep 2025 20:21:52 -0700
Davidlohr Bueso <[email protected]> wrote:
Add basic plumbing for memory expander devices that support Back
Invalidation. This introduces a 'hdm-db=on|off' parameter and
exposes the relevant BI RT/Decoder component cachemem registers.
Some noteworthy properties:
- Devices require enabling Flit mode.
- Explicit BI-ID commit is required.
- HDM decoder support both host and dev coherency models.
Signed-off-by: Davidlohr Bueso <[email protected]>
Hi Davidlohr,
Comments inline mostly focus on the bi parameter. I think flipping
it to true for components where we are hard coding it as true will
move that logic decision up a layer and make the code easier to follow.
Agreed.
...
diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index e5a0d1fb308c..4bc185df8c87 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -136,7 +136,7 @@ static void latch_registers(CXLUpstreamPort *usp)
uint32_t *write_msk = usp->cxl_cstate.crb.cache_mem_regs_write_mask;
cxl_component_register_init_common(reg_state, write_msk,
- CXL2_UPSTREAM_PORT);
+ CXL2_UPSTREAM_PORT, false);
Obviously different structure but still seems odd that bi is false yet we create
BI_RT structure in this call.
I'm thinking maybe use usp->flitmode instead. Yeah it's not ideal to associate
them like that, but it makes less sense to always export the registers if 256B
is not even enabled. Otherwise now this would have to be hard coded to true as
well.
Thanks,
Davidlohr