Re: [PATCH v1 1/2] hw/cxl/device: read from register values in mdev_reg_read()

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Mon, 27 Nov 2023 12:27:02 -0800
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Mon, 27 Nov 2023, Hyeonggon Yoo wrote:
> 
> >In the current mdev_reg_read() implementation, it consistently returns
> >that the Media Status is Ready (01b). This was fine until commit
> >25a52959f99d ("hw/cxl: Add support for device sanitation") because the
> >media was presumed to be ready.
> >
> >However, as per the CXL 3.0 spec "8.2.9.8.5.1 Sanitize (Opcode 4400h)",
> >during sanitation, the Media State should be set to Disabled (11b). The
> >mentioned commit correctly sets it to Disabled, but mdev_reg_read()
> >still returns Media Status as Ready.
> >
> >To address this, update mdev_reg_read() to read register values instead
> >of returning dummy values.
> >
> >Fixes: commit 25a52959f99d ("hw/cxl: Add support for device sanitation")
> >Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>  
> 
> Looks good, thanks.
> 
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
> 
> In addition how about the following to further robustify?
>    - disallow certain incoming cci cmd when media is disabled
>    - deal with memory reads/writes when media is disabled
>    - make __toggle_media() a nop when passed value is already set
>    - play nice with arm64 uses little endian reads and writes (this
>      should be extended to all of mbox/cci of course).
This one you've lost me on.  Arm64 and x86 both little endian.

If you mean generally harden the code we haven't fixed up for
big endian systems then fair enough - that indeed needs doing.
Tricky to be sure we got it all right though unless we have a big
endian arch to test on...

Jonathan