Re: [PATCH 0/1] hw/nvme: create parameter to enable/disable cmic on subsystem

[alan.adamson@oracle.com]

On 4/8/25 11:47 PM, Klaus Jensen wrote:
> On Apr  8 15:56, Alan Adamson wrote:
>> While testing Linux atomic writes with qemu-nvme v10.0.0-rc1, Linux was
>> incorrectly displaying atomic_write_max_bytes
>> # cat /sys/block/nvme0n1/queue/atomic_write_max_bytes
>> 0
>> # nvme id-ctrl /dev/nvme0n1 | grep awupf
>> awupf     : 15
>> #
>> Since AWUPF was set to 15, it was expected atomic_write_max_bytes would
>> be set to 8192.
>>
>> The commit cd59f50ab017 ("hw/nvme: always initialize a subsystem")
>> introduced this behavior. The commit hardcodes the subsystem cmic bit
>> to ON which caused the Linux NVMe driver to treat the namespace as
>> multi-pathed which uncovered a bug with how Atomic Write Queue Limits
>> were being inherited.  This Linux issue is being addressed, but the
>> question was asked of why the subsystem cmic bit was hardcoded to ON.
>> Most NVMe devices today don't set cmic to ON. Shouldn't the setting of
>> this bit be a settable parameter?
>>
>> <subsystem>,cmic=BOOLEAN (default: off)
>>
>> Example:
>>      -device nvme-subsys,id=subsys0,cmic=on \
>>      -device nvme,serial=deadbeef,id=nvme0,subsys=subsys0,atomic.dn=off,atomic.awun=31,atomic.awupf=15 \
>>      -drive id=ns1,file=/dev/nullb3,if=none \
>>      -device nvme-ns,drive=ns1,bus=nvme0,nsid=1,shared=false
>>
>> Alan Adamson (1):
>>    hw/nvme: create parameter to enable/disable cmic on subsystem
>>
>>   hw/nvme/ctrl.c   | 5 ++++-
>>   hw/nvme/nvme.h   | 1 +
>>   hw/nvme/subsys.c | 1 +
>>   3 files changed, 6 insertions(+), 1 deletion(-)
>>
>> -- 
>> 2.43.5
>>
>>
> Hi Alan,
>
> I agree that it would be better for CMIC.MCTRS to remain zero for
> single-controller subsystems, but I'd rather not add the parameter
> without verifying it (it would be invalid to have a multi-controller
> subsystem with CMIC.MCTRS zeroed).
>
> An improvement would be to just set it if the subsystems contains
> multiple controllers.
>
> Prior to commit cd59f50ab017 we would also statically set CMIC.MCTRS to
> 1 if a subsystem was configured, regardless of the number of
> controllers.

Klaus,

I'll send out a v2 that automatically sets CMIC.MCTRS for each 
controller of a multi-controller subsystem.  For single controller 
subsystems, CMIC.MCTRS will default to off and the cmic parameter can be 
used to change CMIC.MCTRS to on.

Thanks!

Alan