* [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries @ 2019-11-11 17:13 Jan Kiszka 2019-11-11 17:17 ` Rob Miller 0 siblings, 1 reply; 8+ messages in thread From: Jan Kiszka @ 2019-11-11 17:13 UTC (permalink / raw) To: virtio-dev@lists.oasis-open.org From: Jan Kiszka <jan.kiszka@siemens.com> So far the spec only indirectly says that a descriptor table entry is not modified by a device when processing it. Make this explicit by adding it as normative requirement. Existing drivers already depend on this. See also https://lists.oasis-open.org/archives/virtio-dev/201910/msg00057.html. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- split-ring.tex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/split-ring.tex b/split-ring.tex index 123ac9f..bfef62d 100644 --- a/split-ring.tex +++ b/split-ring.tex @@ -217,7 +217,7 @@ \subsection{The Virtqueue Descriptor Table}\label{sec:Basic Facilities of a Virt \devicenormative{\subsubsection}{The Virtqueue Descriptor Table}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table} A device MUST NOT write to a device-readable buffer, and a device SHOULD NOT read a device-writable buffer (it MAY do so for debugging or diagnostic -purposes). +purposes). A device MUST NOT write to any descriptor table entry. \drivernormative{\subsubsection}{The Virtqueue Descriptor Table}{Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table} Drivers MUST NOT add a descriptor chain longer than $2^{32}$ bytes in total; -- 2.16.4 --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2019-11-11 17:13 [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries Jan Kiszka @ 2019-11-11 17:17 ` Rob Miller 2019-11-11 17:26 ` Jan Kiszka 2020-01-30 11:50 ` Michael S. Tsirkin 0 siblings, 2 replies; 8+ messages in thread From: Rob Miller @ 2019-11-11 17:17 UTC (permalink / raw) To: virtio-dev@lists.oasis-open.org [-- Attachment #1: Type: text/plain, Size: 1923 bytes --] what is trying to be solved here? There is a reason why this is allowed as some vendors update the table when using RX_MERABLE_BUFFERs & F_IN_ORDER features Rob Miller rob.miller@broadcom.com (919)721-3339 On Mon, Nov 11, 2019 at 12:13 PM Jan Kiszka <jan.kiszka@siemens.com> wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > So far the spec only indirectly says that a descriptor table entry is > not modified by a device when processing it. Make this explicit by > adding it as normative requirement. Existing drivers already depend on > this. > > See also > https://lists.oasis-open.org/archives/virtio-dev/201910/msg00057.html. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > split-ring.tex | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/split-ring.tex b/split-ring.tex > index 123ac9f..bfef62d 100644 > --- a/split-ring.tex > +++ b/split-ring.tex > @@ -217,7 +217,7 @@ \subsection{The Virtqueue Descriptor > Table}\label{sec:Basic Facilities of a Virt > \devicenormative{\subsubsection}{The Virtqueue Descriptor Table}{Basic > Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table} > A device MUST NOT write to a device-readable buffer, and a device SHOULD > NOT > read a device-writable buffer (it MAY do so for debugging or diagnostic > -purposes). > +purposes). A device MUST NOT write to any descriptor table entry. > > \drivernormative{\subsubsection}{The Virtqueue Descriptor Table}{Basic > Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table} > Drivers MUST NOT add a descriptor chain longer than $2^{32}$ bytes in > total; > -- > 2.16.4 > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > > [-- Attachment #2: Type: text/html, Size: 2884 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2019-11-11 17:17 ` Rob Miller @ 2019-11-11 17:26 ` Jan Kiszka 2020-02-27 10:05 ` Michael S. Tsirkin 2020-01-30 11:50 ` Michael S. Tsirkin 1 sibling, 1 reply; 8+ messages in thread From: Jan Kiszka @ 2019-11-11 17:26 UTC (permalink / raw) To: Rob Miller, virtio-dev@lists.oasis-open.org On 11.11.19 18:17, Rob Miller wrote: > what is trying to be solved here? There is a reason why this is allowed > as some vendors update the table when using RX_MERABLE_BUFFERs & > F_IN_ORDER features OK, good to know. Which fields are updated then? Under which conditions (ie. in which states of the device)? The background is in the link below. My goal was to find a simple way declaring all fields read-only for the device to avoid problems with new implementations. But it looks like it's more complicated... Jan > > Rob Miller > rob.miller@broadcom.com <mailto:rob.miller@broadcom.com> > (919)721-3339 > > > On Mon, Nov 11, 2019 at 12:13 PM Jan Kiszka <jan.kiszka@siemens.com > <mailto:jan.kiszka@siemens.com>> wrote: > > From: Jan Kiszka <jan.kiszka@siemens.com > <mailto:jan.kiszka@siemens.com>> > > So far the spec only indirectly says that a descriptor table entry is > not modified by a device when processing it. Make this explicit by > adding it as normative requirement. Existing drivers already depend on > this. > > See also > https://lists.oasis-open.org/archives/virtio-dev/201910/msg00057.html. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com > <mailto:jan.kiszka@siemens.com>> > --- > split-ring.tex | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/split-ring.tex b/split-ring.tex > index 123ac9f..bfef62d 100644 > --- a/split-ring.tex > +++ b/split-ring.tex > @@ -217,7 +217,7 @@ \subsection{The Virtqueue Descriptor > Table}\label{sec:Basic Facilities of a Virt > \devicenormative{\subsubsection}{The Virtqueue Descriptor > Table}{Basic Facilities of a Virtio Device / Virtqueues / The > Virtqueue Descriptor Table} > A device MUST NOT write to a device-readable buffer, and a device > SHOULD NOT > read a device-writable buffer (it MAY do so for debugging or > diagnostic > -purposes). > +purposes). A device MUST NOT write to any descriptor table entry. > > \drivernormative{\subsubsection}{The Virtqueue Descriptor > Table}{Basic Facilities of a Virtio Device / Virtqueues / The > Virtqueue Descriptor Table} > Drivers MUST NOT add a descriptor chain longer than $2^{32}$ bytes > in total; > -- > 2.16.4 > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > <mailto:virtio-dev-unsubscribe@lists.oasis-open.org> > For additional commands, e-mail: > virtio-dev-help@lists.oasis-open.org > <mailto:virtio-dev-help@lists.oasis-open.org> > -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2019-11-11 17:26 ` Jan Kiszka @ 2020-02-27 10:05 ` Michael S. Tsirkin 2020-03-02 12:01 ` Rob Miller 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2020-02-27 10:05 UTC (permalink / raw) To: Jan Kiszka; +Cc: Rob Miller, virtio-dev@lists.oasis-open.org On Mon, Nov 11, 2019 at 06:26:58PM +0100, Jan Kiszka wrote: > On 11.11.19 18:17, Rob Miller wrote: > > what is trying to be solved here? There is a reason why this is allowed > > as some vendors update the table when using RX_MERABLE_BUFFERs & > > F_IN_ORDER features > > OK, good to know. Which fields are updated then? Under which conditions (ie. > in which states of the device)? > > The background is in the link below. My goal was to find a simple way > declaring all fields read-only for the device to avoid problems with new > implementations. But it looks like it's more complicated... > > Jan No answer yet. Rob I see the following options: - proceed with the ballot as is ignoring your comment. Maybe it's all a misunderstanding? It could be the comment was referring to the packed ring, where this patch refers to the split ring. - make it weaker: MUST if PLATFORM_ACCESS is negotiated, and SHOULD if it isn't. > > > > Rob Miller > > rob.miller@broadcom.com <mailto:rob.miller@broadcom.com> > > (919)721-3339 > > > > > > On Mon, Nov 11, 2019 at 12:13 PM Jan Kiszka <jan.kiszka@siemens.com > > <mailto:jan.kiszka@siemens.com>> wrote: > > > > From: Jan Kiszka <jan.kiszka@siemens.com > > <mailto:jan.kiszka@siemens.com>> > > > > So far the spec only indirectly says that a descriptor table entry is > > not modified by a device when processing it. Make this explicit by > > adding it as normative requirement. Existing drivers already depend on > > this. > > > > See also > > https://lists.oasis-open.org/archives/virtio-dev/201910/msg00057.html. > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com > > <mailto:jan.kiszka@siemens.com>> > > --- > > split-ring.tex | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/split-ring.tex b/split-ring.tex > > index 123ac9f..bfef62d 100644 > > --- a/split-ring.tex > > +++ b/split-ring.tex > > @@ -217,7 +217,7 @@ \subsection{The Virtqueue Descriptor > > Table}\label{sec:Basic Facilities of a Virt > > \devicenormative{\subsubsection}{The Virtqueue Descriptor > > Table}{Basic Facilities of a Virtio Device / Virtqueues / The > > Virtqueue Descriptor Table} > > A device MUST NOT write to a device-readable buffer, and a device > > SHOULD NOT > > read a device-writable buffer (it MAY do so for debugging or > > diagnostic > > -purposes). > > +purposes). A device MUST NOT write to any descriptor table entry. > > > > \drivernormative{\subsubsection}{The Virtqueue Descriptor > > Table}{Basic Facilities of a Virtio Device / Virtqueues / The > > Virtqueue Descriptor Table} > > Drivers MUST NOT add a descriptor chain longer than $2^{32}$ bytes > > in total; > > -- 2.16.4 > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > <mailto:virtio-dev-unsubscribe@lists.oasis-open.org> > > For additional commands, e-mail: > > virtio-dev-help@lists.oasis-open.org > > <mailto:virtio-dev-help@lists.oasis-open.org> > > > > -- > Siemens AG, Corporate Technology, CT RDA IOT SES-DE > Corporate Competence Center Embedded Linux > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2020-02-27 10:05 ` Michael S. Tsirkin @ 2020-03-02 12:01 ` Rob Miller 2020-03-02 12:13 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Rob Miller @ 2020-03-02 12:01 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Jan Kiszka, virtio-dev@lists.oasis-open.org [-- Attachment #1: Type: text/plain, Size: 4048 bytes --] In reviewing my comments, I agree that I misunderstood what was being requested. Please ignore my comments and continue as needed. Rob Miller rob.miller@broadcom.com (919)721-3339 On Thu, Feb 27, 2020 at 5:05 AM Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Nov 11, 2019 at 06:26:58PM +0100, Jan Kiszka wrote: > > On 11.11.19 18:17, Rob Miller wrote: > > > what is trying to be solved here? There is a reason why this is allowed > > > as some vendors update the table when using RX_MERABLE_BUFFERs & > > > F_IN_ORDER features > > > > OK, good to know. Which fields are updated then? Under which conditions > (ie. > > in which states of the device)? > > > > The background is in the link below. My goal was to find a simple way > > declaring all fields read-only for the device to avoid problems with new > > implementations. But it looks like it's more complicated... > > > > Jan > > No answer yet. Rob I see the following options: > - proceed with the ballot as is ignoring your comment. Maybe it's all a > misunderstanding? It could be the comment was referring to the > packed ring, where this patch refers to the split ring. > - make it weaker: MUST if PLATFORM_ACCESS is negotiated, and > SHOULD if it isn't. > > > > > > > Rob Miller > > > rob.miller@broadcom.com <mailto:rob.miller@broadcom.com> > > > (919)721-3339 > > > > > > > > > On Mon, Nov 11, 2019 at 12:13 PM Jan Kiszka <jan.kiszka@siemens.com > > > <mailto:jan.kiszka@siemens.com>> wrote: > > > > > > From: Jan Kiszka <jan.kiszka@siemens.com > > > <mailto:jan.kiszka@siemens.com>> > > > > > > So far the spec only indirectly says that a descriptor table entry > is > > > not modified by a device when processing it. Make this explicit by > > > adding it as normative requirement. Existing drivers already > depend on > > > this. > > > > > > See also > > > > https://lists.oasis-open.org/archives/virtio-dev/201910/msg00057.html. > > > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com > > > <mailto:jan.kiszka@siemens.com>> > > > --- > > > split-ring.tex | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/split-ring.tex b/split-ring.tex > > > index 123ac9f..bfef62d 100644 > > > --- a/split-ring.tex > > > +++ b/split-ring.tex > > > @@ -217,7 +217,7 @@ \subsection{The Virtqueue Descriptor > > > Table}\label{sec:Basic Facilities of a Virt > > > \devicenormative{\subsubsection}{The Virtqueue Descriptor > > > Table}{Basic Facilities of a Virtio Device / Virtqueues / The > > > Virtqueue Descriptor Table} > > > A device MUST NOT write to a device-readable buffer, and a device > > > SHOULD NOT > > > read a device-writable buffer (it MAY do so for debugging or > > > diagnostic > > > -purposes). > > > +purposes). A device MUST NOT write to any descriptor table entry. > > > > > > \drivernormative{\subsubsection}{The Virtqueue Descriptor > > > Table}{Basic Facilities of a Virtio Device / Virtqueues / The > > > Virtqueue Descriptor Table} > > > Drivers MUST NOT add a descriptor chain longer than $2^{32}$ > bytes > > > in total; > > > -- 2.16.4 > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: > virtio-dev-unsubscribe@lists.oasis-open.org > > > <mailto:virtio-dev-unsubscribe@lists.oasis-open.org> > > > For additional commands, e-mail: > > > virtio-dev-help@lists.oasis-open.org > > > <mailto:virtio-dev-help@lists.oasis-open.org> > > > > > > > -- > > Siemens AG, Corporate Technology, CT RDA IOT SES-DE > > Corporate Competence Center Embedded Linux > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org > > [-- Attachment #2: Type: text/html, Size: 6385 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2020-03-02 12:01 ` Rob Miller @ 2020-03-02 12:13 ` Michael S. Tsirkin 2020-03-02 12:45 ` Jan Kiszka 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2020-03-02 12:13 UTC (permalink / raw) To: Rob Miller; +Cc: Jan Kiszka, virtio-dev@lists.oasis-open.org On Mon, Mar 02, 2020 at 07:01:37AM -0500, Rob Miller wrote: > In reviewing my comments, I agree that I misunderstood what was being > requested. Please ignore my comments and continue as needed. OK then. Jan should we start the vote? --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2020-03-02 12:13 ` Michael S. Tsirkin @ 2020-03-02 12:45 ` Jan Kiszka 0 siblings, 0 replies; 8+ messages in thread From: Jan Kiszka @ 2020-03-02 12:45 UTC (permalink / raw) To: Michael S. Tsirkin, Rob Miller; +Cc: virtio-dev@lists.oasis-open.org On 02.03.20 13:13, Michael S. Tsirkin wrote: > On Mon, Mar 02, 2020 at 07:01:37AM -0500, Rob Miller wrote: >> In reviewing my comments, I agree that I misunderstood what was being >> requested. Please ignore my comments and continue as needed. > > OK then. Jan should we start the vote? > Fine with me, yes. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries 2019-11-11 17:17 ` Rob Miller 2019-11-11 17:26 ` Jan Kiszka @ 2020-01-30 11:50 ` Michael S. Tsirkin 1 sibling, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2020-01-30 11:50 UTC (permalink / raw) To: Rob Miller; +Cc: virtio-dev@lists.oasis-open.org On Mon, Nov 11, 2019 at 12:17:51PM -0500, Rob Miller wrote: > On Mon, Nov 11, 2019 at 12:13 PM Jan Kiszka <jan.kiszka@siemens.com> wrote: > > From: Jan Kiszka <jan.kiszka@siemens.com> > > So far the spec only indirectly says that a descriptor table entry is > not modified by a device when processing it. Make this explicit by > adding it as normative requirement. Existing drivers already depend on > this. > > See also https://lists.oasis-open.org/archives/virtio-dev/201910/ > msg00057.html. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > split-ring.tex | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/split-ring.tex b/split-ring.tex > index 123ac9f..bfef62d 100644 > --- a/split-ring.tex > +++ b/split-ring.tex > @@ -217,7 +217,7 @@ \subsection{The Virtqueue Descriptor Table}\label > {sec:Basic Facilities of a Virt > \devicenormative{\subsubsection}{The Virtqueue Descriptor Table}{Basic > Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor > Table} > A device MUST NOT write to a device-readable buffer, and a device SHOULD > NOT > read a device-writable buffer (it MAY do so for debugging or diagnostic > -purposes). > +purposes). A device MUST NOT write to any descriptor table entry. > > \drivernormative{\subsubsection}{The Virtqueue Descriptor Table}{Basic > Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor > Table} > Drivers MUST NOT add a descriptor chain longer than $2^{32}$ bytes in > total; > -- > 2.16.4 > > > what is trying to be solved here? There is a reason why this is allowed as some > vendors update the table when using RX_MERABLE_BUFFERs & F_IN_ORDER features > > Rob Miller > rob.miller@broadcom.com > (919)721-3339 > > Going back to this, I don't see how this can work with current Linux guests at least when VIRTIO_F_IOMMU_PLATFORM is specified, since the data is mapped to devices as follows: dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_TO_DEVICE); and I guess we all agreed physical devices all set this flag? Could you explain a bit more about how writing into descriptors is useful with RX_MERABLE_BUFFERs? -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-02 12:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-11 17:13 [virtio-dev] [PATCH] split-ring: Demand that a device must not change descriptor entries Jan Kiszka 2019-11-11 17:17 ` Rob Miller 2019-11-11 17:26 ` Jan Kiszka 2020-02-27 10:05 ` Michael S. Tsirkin 2020-03-02 12:01 ` Rob Miller 2020-03-02 12:13 ` Michael S. Tsirkin 2020-03-02 12:45 ` Jan Kiszka 2020-01-30 11:50 ` Michael S. Tsirkin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.