On Wed, 26 Jan 2022 18:17:12 +0000 Alex Bennée <alex.ben...@linaro.org> wrote:
> Jonathan Cameron <jonathan.came...@huawei.com> writes: > > > From: Ben Widawsky <ben.widaw...@intel.com> > > > > This implements all device MMIO up to the first capability. That > > includes the CXL Device Capabilities Array Register, as well as all of > > the CXL Device Capability Header Registers. The latter are filled in as > > they are implemented in the following patches. > > > > Endianness and alignment are managed by softmmu memory core. > > > > Signed-off-by: Ben Widawsky <ben.widaw...@intel.com> > > Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com> > > --- > > hw/cxl/cxl-device-utils.c | 105 ++++++++++++++++++++++++++++++++++++ > > hw/cxl/meson.build | 1 + > > include/hw/cxl/cxl_device.h | 28 +++++++++- > > 3 files changed, 133 insertions(+), 1 deletion(-) > > > > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c > > new file mode 100644 > > index 0000000000..cb1b0a8217 > > --- /dev/null > > +++ b/hw/cxl/cxl-device-utils.c > > @@ -0,0 +1,105 @@ > > +/* > > + * CXL Utility library for devices > > + * > > + * Copyright(C) 2020 Intel Corporation. > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2. See the > > + * COPYING file in the top-level directory. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "qemu/log.h" > > +#include "hw/cxl/cxl.h" > > + > > +/* > > + * Device registers have no restrictions per the spec, and so fall back to > > the > > + * default memory mapped register rules in 8.2: > > + * Software shall use CXL.io Memory Read and Write to access memory > > mapped > > + * register defined in this section. Unless otherwise specified, software > > + * shall restrict the accesses width based on the following: > > + * • A 32 bit register shall be accessed as a 1 Byte, 2 Bytes or 4 Bytes > > + * quantity. > > + * • A 64 bit register shall be accessed as a 1 Byte, 2 Bytes, 4 Bytes > > or 8 > > + * Bytes > > + * • The address shall be a multiple of the access width, e.g. when > > + * accessing a register as a 4 Byte quantity, the address shall be > > + * multiple of 4. > > + * • The accesses shall map to contiguous bytes.If these rules are not > > + * followed, the behavior is undefined > > + */ > > + > > +static uint64_t caps_reg_read(void *opaque, hwaddr offset, unsigned size) > > +{ > > + CXLDeviceState *cxl_dstate = opaque; > > + > > + return cxl_dstate->caps_reg_state32[offset / 4]; > > +} > > + > > +static uint64_t dev_reg_read(void *opaque, hwaddr offset, unsigned size) > > +{ > > + return 0; > > +} > > + > > +static const MemoryRegionOps dev_ops = { > > + .read = dev_reg_read, > > + .write = NULL, /* status register is read only */ > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .valid = { > > + .min_access_size = 1, > > + .max_access_size = 8, > > + .unaligned = false, > > + }, > > + .impl = { > > + .min_access_size = 1, > > + .max_access_size = 8, > > + }, > > +}; > > I think for >64 bit registers you need to use the read_with_attrs I don't follow this comment. Max access to registers is 64 bits. A few are documented as 128 bit or indeed larger in the spec, but the access is as if they were multiple 64 bit registers accesses. It's not permissible to do a single 128bit access for example. The F4 errata clarified that - previously it was rather unclear what the restrictions on access to the larger registers were. I've updated a few comments on this to reflect the errata. Thanks, Jonathan