> On Jun 15, 2022, at 5:38 PM, Klaus Jensen wrote:
>
> I prefer we use the NVMe terminology to minimize misunderstandings, so
> "host" means the driver and "device" means the qemu side of things
>
Thanks for helping me disambiguate this!
Now that we have resolved all issues in v1, I’ve subm
On Wed, Jun 15, 2022 at 07:22:22PM +0800, Jinhao Fan wrote:
> >>> Isn't this racy against the driver? Compare
> >>> https://github.com/spdk/spdk/blob/master/lib/nvmf/vfio_user.c#L1317
> >>
> >> QEMU has full memory barriers on dma read/write, so I believe this is
> >> safe?
> >
> > But don't you
> On Jun 15, 2022, at 6:11 PM, John Levon wrote:
>
> On Wed, Jun 15, 2022 at 11:33:02AM +0200, Klaus Jensen wrote:
>
>>> BTW I'm surprised that this patch has just this:
>>>
>>> +static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
>>> +{
>>> +pci_dma_write(&sq->ctrl->parent_obj, sq
On Wed, Jun 15, 2022 at 11:33:02AM +0200, Klaus Jensen wrote:
> > BTW I'm surprised that this patch has just this:
> >
> > +static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
> > +{
> > +pci_dma_write(&sq->ctrl->parent_obj, sq->ei_addr, &sq->tail,
> > + sizeof(sq->tail
On Jun 15 11:58, Jinhao Fan wrote:
>
> > On Jun 14, 2022, at 11:41 PM, Keith Busch wrote:
> >
> > It's a pretty nasty hack, and definitely not in compliance with the spec:
> > the
> > db_addr is supposed to be read-only from the device side, though I do think
> > it's safe for this environment.
On Jun 15 10:07, John Levon wrote:
> On Wed, Jun 15, 2022 at 10:48:26AM +0200, Klaus Jensen wrote:
>
> > > By the way, I noticed that the patch never updates the cq's ei_addr
> > > value. Is
> > > that on purpose?
> >
> > Yeah, I also mentioned this previously[1] and I still think we need to
> >
On Wed, Jun 15, 2022 at 10:48:26AM +0200, Klaus Jensen wrote:
> > By the way, I noticed that the patch never updates the cq's ei_addr value.
> > Is
> > that on purpose?
>
> Yeah, I also mentioned this previously[1] and I still think we need to
> update the event index. Otherwise (and my testing
On Jun 14 08:41, Keith Busch wrote:
> It's a pretty nasty hack, and definitely not in compliance with the spec: the
> db_addr is supposed to be read-only from the device side, though I do think
> it's safe for this environment. Unless Klaus or anyone finds something I'm
> missing, I feel this is an
> On Jun 14, 2022, at 11:41 PM, Keith Busch wrote:
>
> It's a pretty nasty hack, and definitely not in compliance with the spec: the
> db_addr is supposed to be read-only from the device side, though I do think
> it's safe for this environment. Unless Klaus or anyone finds something I'm
> missi
On Tue, Jun 14, 2022 at 03:24:37PM +0800, Jinhao Fan wrote:
> > On Jun 14, 2022, at 5:15 AM, Keith Busch wrote:
> > @@ -6538,9 +6544,25 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr
> > addr, int val)
> >
> > trace_pci_nvme_mmio_doorbell_sq(sq->sqid, new_tail);
> >
> > -if
> On Jun 14, 2022, at 5:15 AM, Keith Busch wrote:
>
>
> @@ -6538,9 +6544,25 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr,
> int val)
>
> trace_pci_nvme_mmio_doorbell_sq(sq->sqid, new_tail);
>
> -if (!sq->db_addr) {
> sq->tail = new_tail;
> +if (sq
On Sun, Jun 12, 2022 at 07:40:55PM +0800, Jinhao Fan wrote:
>
> > On Jun 10, 2022, at 1:27 AM, Klaus Jensen wrote:
> >
> > I'm ok with following the concensus here, but we all agree that this is
> > a blatant spec violation that ended up manifesting itself down the
> > stack, right?
> >
> > So.
> On Jun 10, 2022, at 1:27 AM, Klaus Jensen wrote:
>
> I'm ok with following the concensus here, but we all agree that this is
> a blatant spec violation that ended up manifesting itself down the
> stack, right?
>
> So... if QEMU wants to be compliant here, I guess we could ask the
> kernel to
On Thu, Jun 09, 2022 at 07:27:57PM +0200, Klaus Jensen wrote:
> > It's not just unnecessary, but enabling shadow doorbells on admin queues
> > will
> > actively break device implementations (such as SPDK), which have had to
> > presume
> > that driver implementations don't use shadow doorbells f
On Jun 9 16:52, John Levon wrote:
> On Thu, Jun 09, 2022 at 08:29:30AM -0600, Keith Busch wrote:
>
> > On Wed, Jun 08, 2022 at 10:55:30PM +0200, Klaus Jensen wrote:
> > >
> > > Keith, is this a bug in the kernel? If the code here would expect the
> > > doorbell buffer to be updated for the admin
On Thu, Jun 09, 2022 at 08:29:30AM -0600, Keith Busch wrote:
> On Wed, Jun 08, 2022 at 10:55:30PM +0200, Klaus Jensen wrote:
> >
> > Keith, is this a bug in the kernel? If the code here would expect the
> > doorbell buffer to be updated for the admin queue as well, would we
> > break?
>
> The ad
On Wed, Jun 08, 2022 at 10:55:30PM +0200, Klaus Jensen wrote:
>
> Keith, is this a bug in the kernel? If the code here would expect the
> doorbell buffer to be updated for the admin queue as well, would we
> break?
The admin queue has to be created before db buffer can be set up, so we can't
rely
> On Jun 9, 2022, at 4:55 AM, Klaus Jensen wrote:
>
> On Jun 8 09:36, Jinhao Fan wrote:
>> Implement Doorbel Buffer Config command (Section 5.7 in NVMe Spec 1.3)
>> and Shadow Doorbel buffer & EventIdx buffer handling logic (Section 7.13
>> in NVMe Spec 1.3). For queues created before the Door
On Jun 8 09:36, Jinhao Fan wrote:
> Implement Doorbel Buffer Config command (Section 5.7 in NVMe Spec 1.3)
> and Shadow Doorbel buffer & EventIdx buffer handling logic (Section 7.13
> in NVMe Spec 1.3). For queues created before the Doorbell Buffer Config
> command, the nvme_dbbuf_config function
Implement Doorbel Buffer Config command (Section 5.7 in NVMe Spec 1.3)
and Shadow Doorbel buffer & EventIdx buffer handling logic (Section 7.13
in NVMe Spec 1.3). For queues created before the Doorbell Buffer Config
command, the nvme_dbbuf_config function tries to associate each existing
SQ and CQ
20 matches
Mail list logo
