Re: [Xen-devel] [PATCH v2 2/2] VT-d: Fix vt-d flush timeout issue.

["Xu, Quan" <quan.xu@xxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx
> [mailto:xen-devel-bounces@xxxxxxxxxxxxx] On Behalf Of Xu, Quan
> Sent: Friday, December 11, 2015 10:00 PM
> To: Jan Beulich
> Cc: Tian, Kevin; Wu, Feng; george.dunlap@xxxxxxxxxxxxx;
> andrew.cooper3@xxxxxxxxxx; tim@xxxxxxx; xen-devel@xxxxxxxxxxxxx; Nakajima,
> Jun; keir@xxxxxxx
> Subject: Re: [Xen-devel] [PATCH v2 2/2] VT-d: Fix vt-d flush timeout issue.
> 
> On 11.12.2015 at 6:05pm, <JBeulich@xxxxxxxx> wrote:
> > >>> On 11.12.15 at 09:01, <quan.xu@xxxxxxxxx> wrote:
> > > On 11.12.2015 at 3:28pm, <Tian, Kevin> wrote:
> > >> > From: Xu, Quan
> > >> > Sent: Thursday, December 10, 2015 5:33 PM
> > >> >
> > >> > If IOTLB/Context/IETC flush is timeout, we should think all
> > >> > devices under this IOMMU cannot function correctly.
> > >> > So for each device under this IOMMU we'll mark it as unassignable
> > >> > and kill the domain owning the device.
> > >> >
> > >> > If Device-TLB flush is timeout, we'll mark the target ATS device
> > >> > as unassignable and kill the domain owning this device.
> > >> >
> > >> > If impacted domain is hardware domain, just throw out a warning.
> > >> > It's an open here whether we want to kill hardware domain (or
> > >> > directly panic hypervisor). Comments are welcomed.
> > >> >
> > >> > Device marked as unassignable will be disallowed to be further
> > >> > assigned to any domain.
> > >> >
> > >> > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> > >> > ---
> > >> [...]
> > >> > diff --git a/xen/drivers/passthrough/vtd/iommu.h
> > >> > b/xen/drivers/passthrough/vtd/iommu.h
> > >> > index ac71ed1..c3beaa6 100644
> > >> > --- a/xen/drivers/passthrough/vtd/iommu.h
> > >> > +++ b/xen/drivers/passthrough/vtd/iommu.h
> > >> > @@ -452,6 +452,11 @@ struct qinval_entry {
> > >> >
> > >> >  #define RESERVED_VAL        0
> > >> >
> > >> > +#define INVALID_DID    ((u16)~0)
> > >> > +#define INVALID_SEG    ((u16)~0)
> > >> > +#define INVALID_BUS    ((u8)~0)
> > >> > +#define INVALID_DEVFN  ((u8)~0)
> > >> > +
> > >>
> > >> Are those invalid values defined by specification?
> > >  This is not defined by specification.
> > >
> > >>Or if they are software
> > >> defined, does related mgmt. code guarantee that they won't be allocated?
> > >>
> > >
> > > As similar as the other Xen code, it defined invalid value with "~0".
> > > Such
> > > as:
> > >           $#define INVALID_MFN (~0UL)
> > >           $#define INVALID_GFN (~0UL)
> > >           .etc
> > >
> > > Code can't not guarantee that won't be allocated, but it can
> > > guarantee it will not be used when it is INVALID_*.
> > > Any idea, how to indicate that the value is invalid?
> >
> > Some other means is needed (be creative). Comparing with
> > INVALID_{MFN,GFN} is bogus, since frame numbers truly can't reach this
> > big a value (there being just
> > 52 bits in physical addresses, i.e. 40 bits in a frame number).
> 
> Jan, thanks for your comments.
> I think I can't use INVALID_* in my patch any more. If I can separate
> invalidate_timeout() into 2 Functions. then I can ignore these INVALID_*
> parameters.
> 
> i.e.
> separate INVALID_* parameters. ignore these INVALID_* parameters.
> void invalidate_timeout(struct iommu *iommu, int type, u16 did, u16 seg, u8
> bus, u8 devfn)
> 
> into
> 
> invalidate_timeout(struct iommu *iommu) and
> device_tlb_invalidate_timeout(struct iommu *iommu, u16 did, u16 seg, u8 bus,
> u8 devfn)
> 
> 
> 
> invalidate_timeout() is for iotlb/iec/context flush error.
> device_tlb_invalidate_timeout is for Device-TLB flush error.
> 
> Then ignore these INVALID_* parameters.
> 
> Right?
> 
> Quan
> 
> 


Correct it.

 i.e.
 separate
void invalidate_timeout(struct iommu *iommu, int type, u16 did, u16 seg, u8
bus, u8 devfn)
 
 into
 
 invalidate_timeout(struct iommu *iommu) 

and

 device_tlb_invalidate_timeout(struct iommu *iommu, u16 did, u16 seg, u8 bus,
 u8 devfn)
  
 invalidate_timeout() is for iotlb/iec/context flush error.
 device_tlb_invalidate_timeout() is for Device-TLB flush error.


-Quan









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