Re: [Qemu-devel] [PATCH v3 1/3] IOMMU: add VTD_CAP_CM to vIOMMU capability exposed to guest

[Jan Kiszka]

On 2016-06-02 15:00, Alex Williamson wrote:
> On Thu, 2 Jun 2016 16:44:39 +0800
> Peter Xu <address@hidden> wrote:
> 
>> On Sat, May 21, 2016 at 06:42:03PM +0200, Jan Kiszka wrote:
>>> On 2016-05-21 18:19, Aviv B.D wrote:  
>>>> From: "Aviv Ben-David" <address@hidden>
>>>>
>>>> This flag tells the guest to invalidate tlb cache also after unmap 
>>>> operations.
>>>>
>>>> Signed-off-by: Aviv Ben-David <address@hidden>
>>>> ---
>>>>  hw/i386/intel_iommu.c          | 3 ++-
>>>>  hw/i386/intel_iommu_internal.h | 1 +
>>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>>> index 347718f..1af8da8 100644
>>>> --- a/hw/i386/intel_iommu.c
>>>> +++ b/hw/i386/intel_iommu.c
>>>> @@ -1949,7 +1949,8 @@ static void vtd_init(IntelIOMMUState *s)
>>>>      s->iq_last_desc_type = VTD_INV_DESC_NONE;
>>>>      s->next_frcd_reg = 0;
>>>>      s->cap = VTD_CAP_FRO | VTD_CAP_NFR | VTD_CAP_ND | VTD_CAP_MGAW |
>>>> -             VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS;
>>>> +             VTD_CAP_SAGAW | VTD_CAP_MAMV | VTD_CAP_PSI | VTD_CAP_SLLPS |
>>>> +             VTD_CAP_CM;  
>>>
>>> Again, needs to be optional because not all guests will support it or
>>> behave differently when it's set (I've one that refuses to work).  
>>
>> There should be more than one way to make it optional. Which is
>> better? What I can think of:
>>
>> (Assume we have Marcel's "-device intel_iommu" working already)
>>
>> 1. Let the CM bit optional, or say, we need to specify something like
>>    "-device intel_iommu,cmbit=on" or we will disable CM bit. If we
>>    have CM disabled but with VFIO device, let QEMU raise error.
>>
>> 2. We automatically detect whether we need CM bit. E.g., if we have
>>    VFIO and vIOMMU both enabled, we automatically set the bit. Another
>>    case is maybe we would in the future support nested vIOMMU? If so,
>>    we can do the same thing for the nested feature.
> 
> 
> Why do we need to support VT-d for guests that do not support CM=1?
> The VT-d spec indicates that software should be written to handle both
> caching modes (6.1).  Granted this is a *should* and not a *must*,
> but can't we consider guests that do not support CM=1 incompatible with
> emulated VT-d?  If CM=0 needs to be supported then we need to shadow
> all of the remapping structures since vfio effectively becomes a cache
> of the that would otherwise depend on the invalidation of both present
> and non-present entries.  What guests do not support CM=1?  Thanks,

- there is at least one guest that does not support CM=1 yet (Jailhouse)
- there might be more or there might be broken ones as hardware
  generally doesn't have CM=1, thus this case is typically untested
- an AMD IOMMU (to my current understanding) will require shadowing
  anyway has it has no comparable concept, thus we will eventually be
  able to use that strategy also for VT-d

Jan

signature.asc
Description: OpenPGP digital signature