Re: [Xen-devel] [PATCH] Fixing ATS enabling for device assignment

[Weidong Han <weidong.han@xxxxxxxxx>]

Jan Beulich wrote:
> > > > Weidong Han  05/19/10 11:12 AM >>>
> > > >         
> > Jan Beulich wrote:
> >     
> > > > > > "Han, Weidong"  05/19/10 4:37 AM >>>
> > > > > >         
> > > > > >             
> > > > Currently, Xen only enables ATS in Xen booting. When a ATS capable device is 
> > > > assigned to guest, ATS is actually not enabled because FLR before assignment 
> > > > causes it to be disabled. Thus ATS cannot be used in guest. This patch enables 
> > > > ATS in domain_context_mapping. This ensures ATS is enabled in assignment 
> > > > because FLR is earlier than domain_context_mapping call. Therefore ATS can be 
> > > > used in guest. This patch also implements disable_ats_device to disable ATS 
> > > > when the device is deassigned from a domain.
> > > >     
> > > >         
> > > Hmm, this looks inconsistent to me: Iirc FLR is being done from the tools, so 
> > > unless it is technically impossible to do FLR after domain_context_mapping(), 
> > > the patch creates a hypervisor dependency on the tools (if nothing else, a 
> > > latent bug - to be introduced the moment operation ordering changes in the 
> > > tools).
> > >   
> > >       
> > It doesn't depend on FLR from tools. It just ensures ATS is enabled for 
> > device assignment in guest. If ATS is already enabled, enable_ats_device 
> > won't re-enable it.
> >     
> Perhaps I didn't express my concern properly: It's not really the hypervisor 
> depending on anything done by the tools, but the code you ad to the hypervisor 
> will trun out pointless if the tools move FLR past the point where 
> domain_context_mapping() gets executed.
Yes, it's a problem in your case. But FLR is required to be earlier than 
domain_context_mapping for device assignment.
>  It would seem more clean to me if e.g. the tools re-enabled ATS after doing 
> FLR if it was enabled before.I
>   
Sounds good. But i'm not sure if it's suitable or easy to maintain 
capability enabling/disabling in control panel, I feel current device 
handling logic (hotplug, assign/deassign, PV and hvm) in control panel 
is a bit of complex.
>   
> > > Further, if ATS gets enabled at boot time, why would you want to disable it 
> > > after de-assignment?
> > >   
> > >       
> > ATS enabled devices will be added to ats_devices list. disabling ATS 
> > will remove a device from that list and avoid leak. If ATS is enabled on 
> > a VF, and it's destroyed by removing PF driver, disable_ats_device will 
> > be invoked to remove it from the list.
> >     
> That's fine, but doesn't answer my question.
>   
if not disable it after de-assignment, enable_ats_device will add the 
device multiple times to ats_devices list.
Regards,
Weidong
> Jan
>
>   

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel