On 6/5/19 3:47 AM, Maxim Levitsky wrote:
> On Mon, 2019-06-03 at 18:25 -0400, John Snow wrote:
>>
>> On 4/17/19 3:53 PM, Maxim Levitsky wrote:
>>> Phase bits are only set by the hardware to indicate new completions
>>> and not by the device driver.
>>>
>>> Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
>>> ---
>>> block/nvme.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/block/nvme.c b/block/nvme.c
>>> index 0684bbd077..2d208000df 100644
>>> --- a/block/nvme.c
>>> +++ b/block/nvme.c
>>> @@ -340,8 +340,6 @@ static bool nvme_process_completion(BDRVNVMeState *s,
>>> NVMeQueuePair *q)
>>> qemu_mutex_lock(&q->lock);
>>> c->cid = cpu_to_le16(0);
>>> q->inflight--;
>>> - /* Flip Phase Tag bit. */
>>> - c->status = cpu_to_le16(le16_to_cpu(c->status) ^ 0x1);
>>> progress = true;
>>> }
>>> if (progress) {
>>>
>>
>> Since you've not got much traction on this and you've pinged a v2, can
>> you point me to a spec or a reproducer that illustrates the problem?
>>
>> (Or wait for more NVME knowledgeable people to give you a review...!)
>
> "A Completion Queue entry is posted to the Completion Queue when the
> controller write of that Completion
> Queue entry to the next free Completion Queue slot inverts the Phase Tag (P)
> bit from its previous value
> in memory. The controller may generate an interrupt to the host to indicate
> that one or more Completion
> Queue entries have been posted."
>
In the future, please reference the sections in your commit messages
when relevant:
NVM Express 1.3, Section 4.1 "Submission Queue & Completion Queue
Definition"
I also found 4.6 "Completion Queue Entry" to be informative; especially
Figure 28 which defines the phase bit.
So: This looks right; does this fix a bug that can be observed? Do we
have any regression tests for block/NVMe?
--js
>
>
> Best regards,
> Maxim Levitsky
>
