Re: [Xen-devel] [PATCH v3 0/2] VT-d flush issue

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, December 21, 2015 9:23 PM
> To: Xu, Quan <quan.xu@xxxxxxxxx>
> Cc: 'andrew.cooper3@xxxxxxxxxx' <andrew.cooper3@xxxxxxxxxx>;
> 'george.dunlap@xxxxxxxxxxxxx' <george.dunlap@xxxxxxxxxxxxx>; Wu, Feng
> <feng.wu@xxxxxxxxx>; Nakajima, Jun <jun.nakajima@xxxxxxxxx>; Tian, Kevin
> <kevin.tian@xxxxxxxxx>; 'xen-devel@xxxxxxxxxxxxx' <xen-devel@xxxxxxxxxxxxx>;
> 'keir@xxxxxxx' <keir@xxxxxxx>; 'tim@xxxxxxx' <tim@xxxxxxx>
> Subject: RE: [Xen-devel] [PATCH v3 0/2] VT-d flush issue
> 
> >>> On 21.12.15 at 14:08, <quan.xu@xxxxxxxxx> wrote:
> >>  On 21.12.2015 at 8:50pm, <JBeulich@xxxxxxxx> wrote:
> >> >>> On 21.12.15 at 13:28, <quan.xu@xxxxxxxxx> wrote:
> >> > On 21.12.2015 at 7:47pm, <JBeulich@xxxxxxxx> wrote:
> >> >> >>> On 20.12.15 at 14:57, <quan.xu@xxxxxxxxx> wrote:
> >> >> > 2. If VT-d is bug, does the hardware_domain continue to work with
> >> >> > PCIe Devices / DRAM well with DMA remapping error?
> >> >> >    I think it is no. furthermore, i think VMM can NOT run a normal
> >> >> > HVM domain without device-passthrough.
> >> >>
> >> >> In addition to what Andrew said - VT-d is effectively not in use for
> >> >> domains without PT device.
> >> >
> >> > IMO, When VT-d is enabled, but is not working correct. These PCI-e
> >> > devices
> >> > (Disks/NICs..) DMA/Interrupt behaviors are not predictable.
> >> > Assumed that, VT-d is effectively not in use for domains without PT
> >> > device, while at least the virtualization infrastructure is not trusted.
> >> > I think it is also not secure to run PV domains.
> >> >
> >> >> Impacting all such domains by crashing the hypervisor just because
> >> >> (in the extreme case) a single domain with PT devices exhibited a
> >> >> flush issue is a no-go imo.
> >> >>
> >> >
> >> > IMO, a VT-d (IEC/Context/Iotlb) flush issue is not a single domain
> >> > behavior, it is a Hypervisor and infrastructure issue.
> >> > ATS device's Device-TLB flush is a single domain issue.
> >> > Back to our original goal, my patch set is for ATS flush issue. right?
> >>
> >> You mean you don't like this entailing clean up of other code?
> >
> >  Jan, for ARM/AMD, I really have no knowledge to fix it. and I have no
> >  ARM/AMD hardware to verify it. if I need to fix these common part of
> > INTEL/ARM/AMD, I think I need to make
> >  Xen compile correct and not to destroy the logic.
> 
> You indeed aren't expected to fix AMD or ARM code, but it may be
> necessary to adjust that code to make error propagation work.
> 
> >> I'm sorry, but I'm
> >> afraid you won't get away without - perhaps the VT-d maintainers could help
> >> here, but in the end you have to face that it was mainly Intel people who
> >> introduced the code which now needs fixing up, so I consider it not exactly
> >> unfair for you (as a
> >> company) to do this work.
> >>
> >
> > Furthermore, I found out that
> >      if IEC/Iotlb/Context flush error, then panic.
> >      Else if device-tlb flush error, we'll hide the target ATS device and
> > kill the domain owning this ATS device. If impacted domain is hardware
> > domain, just throw out a warning.
> >
> >      Then, it is fine to _not_check all the way up the device-tlb flush call
> > trees( maybe it is our next topic of discussion).
> 
> I don't follow - this sounds more or less like the model you've been
> following in past versions, yet it was that which prompted the
> request to properly propagate errors.

Maybe, there are still some misunderstanding about your expectation.
Let me summarize it here.

After Quan's patch-set, there are two types of error code:
- -EOPNOTSUPP
Now we only support and use software way to synchronize the invalidation,
if someone calls queue_invalidate_wait() and passes sw with 0, then
-EOPNOTSUPP is returned (Though this cannot happen in real world, since 
queue_invalidate_wait() is called only in one place and 1 is passed in to 'sw').
So I am not sure what should we do for this return value, if we really get
that return value, it means the flush is not actually executed, so the iommu
state is incorrect, the data is inconsistent. Do you think what should we do
for this case?

- -ETIMEDOUT
For this case, Quan has elaborate a lot, IIUIC, the main gap between you
and Quan is you think the error code should be propagated to the up caller,
while in Quan's implementation, he deals with this error in invalidate_timeout()
and device_tlb_invalidate_timeout(), hence no need to propagated it to
up called, since the handling policy will crash the domain, so we don't think
propagated error code is needed. Even we propagate it, the up caller still
doesn't need to do anything for it.

Quan, please correct me if I am wrong.

Thanks,
Feng

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel