Re: [virtio-comment] [PATCH] iommu: offered -> negotiated

[Harald Mommer <hmo@opensynergy.com>]

Hello,

I consider those changes not as clarifications but as requirements 
changes which will cause software to be changed.

On 12.04.22 10:30, Cornelia Huck wrote:
> All those clauses actually apply whenever the feature is negotiated,
> not merely offered. Rename to clarify things.
>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>   virtio-iommu.tex | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/virtio-iommu.tex b/virtio-iommu.tex
> index f8cbe8895f0d..efe1000ac2d2 100644
> --- a/virtio-iommu.tex
> +++ b/virtio-iommu.tex
> @@ -232,22 +232,22 @@ \subsection{Device operations}\label{sec:Device Types / IOMMU Device / Device op
>     Creating mappings aligned on large page sizes can improve performance
>     since they require fewer page table and TLB entries.
>   
> -\item If the VIRTIO_IOMMU_F_DOMAIN_RANGE feature is offered,
> +\item If the VIRTIO_IOMMU_F_DOMAIN_RANGE feature is negotiated,
>     \field{domain_range} describes the values supported in a \field{domain}
> -  field. If the feature is not offered, any \field{domain} value is valid.
> +  field. If the feature is not negotiated, any \field{domain} value is valid.
>   
> -\item If the VIRTIO_IOMMU_F_INPUT_RANGE feature is offered,
> +\item If the VIRTIO_IOMMU_F_INPUT_RANGE feature is negotiated,
>     \field{input_range} contains the virtual address range that the IOMMU is
>     able to translate. Any mapping request to virtual addresses outside of
>     this range fails.
>   
> -  If the feature is not offered, virtual mappings span over the whole
> +  If the feature is not negotiated, virtual mappings span over the whole
>     64-bit address space (\texttt{start = 0, end = 0xffffffff ffffffff})
>   \end{itemize}

Assume the following situation: Due to internal implementation there is 
a device which can only work when the domain range from the driver is in 
a certain value range (internal implementation: Array of N domains 
indexed from 0 to N-1). And then there is a driver which behaves in a 
compliant manner but has no idea about the feature 
VIRTIO_IOMMU_F_DOMAIN_RANGE so does not negotiate it. With "negotiated" 
the driver the device has to fail feature negotiation as it cannot 
accept all possible domain values during live operation. With "offered" 
it can succeed feature negotiation as it reserves the right to fail 
later in the ATTACH/DETACH/MAP/UNMAP REQUEST with error 
VIRTIO_IOMMU_S_RANGE when a domain value is received it cannot handle 
internally.

Exactly the same applies for VIRTIO_IOMMU_F_INPUT_RANGE.

The implementation I'm doing currently cannot work with all possible 
virtual address value ranges due to efficiency reasons. With the changed 
text the implementation should also be changed now to fail feature 
negotiation if the driver does not offer VIRTIO_IOMMU_F_INPUT_RANGE. As 
the implementation can currently handle all possible domain values (not 
in the future for efficiency reasons when the implementation is getting 
optimized) I can now think about failing the feature negotiation if 
VIRTIO_IOMMU_F_DOMAIN_RANGE was not negotiated or to remove the checks 
for the allowed value range already implemented.

No big issue (for me). But causes a change in the implementation and may 
therefore not be an editorial change only. The original wording may not 
have been a mistake from the author. It may have been there for an 
(unknown) purpose. Some old driver known to behave compliant to the 
device implementation but having no idea about those features?

Regards
Harald

--
Dipl.-Ing. Harald Mommer
Senior Software Engineer

OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin

Phone:  +49 (30) 60 98 540-0 <== Zentrale
Fax:    +49 (30) 60 98 540-99
E-Mail: harald.mommer@opensynergy.com

www.opensynergy.com

Handelsregister: Amtsgericht Charlottenburg, HRB 108616B
GeschÃftsfÃhrer/Managing Director: Regis Adjamah