[PATCH 2/3] nvmet: pci-epf: Clear CC and CSTS when disabling the controller

[Damien Le Moal]

On 4/10/25 17:34, Keith Busch wrote:
> On Tue, Apr 08, 2025 at 11:47:32AM +0900, Damien Le Moal wrote:
>> @@ -1895,6 +1912,11 @@ static void nvmet_pci_epf_disable_ctrl(struct nvmet_pci_epf_ctrl *ctrl)
>>  	nvmet_pci_epf_delete_cq(ctrl->tctrl, 0);
>>  
>>  	ctrl->csts &= ~NVME_CSTS_RDY;
>> +	if (shutdown) {
>> +		ctrl->csts |= NVME_CSTS_SHST_CMPLT;
>> +		ctrl->cc &= ~NVME_CC_ENABLE;
>> +		nvmet_pci_epf_bar_write32(ctrl, NVME_REG_CC, ctrl->cc);
>> +	}
>>  }
> 
> I think this is probably okay, but I don't know if it's necessary to be
> messing with CC.EN that the host didn't request. The qemu emulated nvme
> doesn't do this, at least. But it looks like that would all work out in
> the end anyway, so again, I think it's fine.

Maybe qemu clears/resets all the registers of the BAR when it does a shutdown ?
I did not check the code. But here, if we do not clear EN, *and* the host does
not first disable the controller before trying to re-enable it (like the linux
host pci driver does), after the shutdown, we fail to detect that the host set
EN again since we can only detect that by polling and comparing to the previous
value.

I also thought about using ctrl->enabled for determining what needs to be done,
but then again, if we do not clear EN from the register, the next CC poll loop
will see EN set and think that it is the host trying to re-enable the controller
and we will re-enable right away.

I think that clearing EN is not necessary with qemu because there is no polling,
since (I think) that qemu nvme driver will be looking at CC.EN only and only
when the guest touches it (as that will trigger a guest exit into the
hypervisor). So the qemu nvme driver always sees the newest value of CC, not the
old one like we do with the endpoint.


-- 
Damien Le Moal
Western Digital Research