Re: [Xen-devel] [PATCH] x86/mtrr: recalculate P2M type for domains with iocaps

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 25/04/2019 16:49, Jan Beulich wrote:
>>>> On 25.04.19 at 16:50, <igor.druzhinin@xxxxxxxxxx> wrote:
>> On 25/04/2019 14:30, Jan Beulich wrote:
>>>>>> On 23.04.19 at 13:37, <igor.druzhinin@xxxxxxxxxx> wrote:
>>>>  void memory_type_changed(struct domain *d)
>>>>  {
>>>> -    if ( has_iommu_pt(d) && d->vcpu && d->vcpu[0] )
>>>> +    if ( (has_iommu_pt(d) || cache_flush_permitted(d)) && d->vcpu && 
>>>> d->vcpu[0] )
>>>>      {
>>>>          p2m_memory_type_changed(d);
>>>>          flush_all(FLUSH_CACHE);
>>>
>>> Wouldn't cache_flush_permitted() alone suffice, both here and
>>> there? Even if anyone was to pass-through a device without any
>>> MMIO or I/O port BAR, the memory type (which is a CPU side
>>> thing only, i.e. doesn't affect DMA by the device) shouldn't
>>> matter in that case (leaving aside the fact that a BAR-less
>>> device is unlikely to be DMA-capable, unless the programming
>>> of the DMA operations was to happen through vendor specific
>>> config space accesses).
>>
>> I don't think it's correct in case of EPT sharing with IOMMU as the
>> section 3.6.5 of VT-d spec implies there is direct effect of caching
>> information present in IOMMU page table on device accesses within
>> processor coherency domain. So even in case there are no BARs passed to
>> the domain, drivers inside it and devices operation on its memory could
>> still have assumptions on device memory access properties.
> 
> But that's a First-Level Translation subsection. For us right now
> section 3.7.4 is relevant, while in the future 3.8.4 may become
> relevant too. As per 3.7.4 all leaf page (actual data) accesses
> are of WB type anyway.
> 
> Once 3.8.4 becomes applicable, we'd likely have to support IOMMU
> side MTRRs as well, or else the MTRR imposed memory type would
> be uniformly UC, allowing for only UC and WC effective memory
> types.
> 

Oh yes, sorry that I mixed them up. In that case, probably yes - simply
cache_flush_permitted() would suffice. Although there are things that
I'm not sure about:
- whether iocaps are always used by software in Dom0 when passing
through certain physical memory and whether their usage is enforced by Xen
- there are could be assumptions elsewhere that with IOMMU context
assigned guest caching choice is in effect - since that fact has been
perpetuated in Xen code for very long time

Igor

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel