Re: [Xen-devel] [PATCH v4 2/2] iommu: add rmrr Xen command line option for misc rmrrs

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx> 05/06/15 6:50 PM >>>
>On Tue, May 05, 2015 at 11:14:30AM +0100, Jan Beulich wrote:
>> >>> On 28.04.15 at 01:50, <elena.ufimtseva@xxxxxxxxxx> wrote:
>> > +        if ( mfn_valid(rmrru->base_address << PAGE_SHIFT) ||
>> > +             mfn_valid(rmrru->end_address << PAGE_SHIFT) )
>> 
>> Please use PFN_UP() and PFN_DOWN() as appropriate. Also, how
>> large do we expect custom RMRRs to reasonably get? If no more
>> than a few pages, I think it would be quite reasonable to check
>> all involved pages here rather than just first and last one. (And
>> even if there can be many pages here, leaving at least a comment
>> saying that this could do with improvement would be nice.)
>
>Hm, thats a hard part not having these regions documented.                     
> 
>But I have found this thread  https://lkml.org/lkml/2014/6/18/683.             
> 
>If I am reading it right, for GPU that region maybe as big as 1024M.           
> 
>For USB I think its smaller, but hard to prove without docs.                   
> 
>I think we should just limit it to some max amount per RMRR and document       
> 
>it. What you think?

Real RMRRs can be basically unlimited, but I'm not convinced we need them
to be unlimited in this workaround. In particular I think we shouldn't aim at
working around GPU related deficiencies of the firmware (at least until we
know we absolutely need to), and hence yes, documenting a limit of a couple
of pages (say 16 for starters) per region would seem reasonable.

>> But what I'm worried about is invalid uses like
>> 
>> rmrr=<addr>=0001:bb:dd.f,0000:bb:dd.f
>> 
>> (as opposed to the valid
>> 
>> rmrr=<addr>=0001:bb:dd.f,bb:dd.f
>> 
>> ). On an earlier version I had specifically pointed out that dealing
>> with an omitted vs a zero segment requires special care.
>
>Yes, you did.                                                                  
> 
>This is just a very easy way to verify segments by making sure                 
> 
>they are all equal if multiple devices specified with minimum code             
> 
>change.                                                                        
> 
>But that can be done in other way as well.                                     
> 
>In your proposed second valid case, segment 0001 should be assumed for         
> 
>second device as its own segment is not specified? 

Exactly. Or (less optimal for the user, but easier to implement) require the 
segment
number to repeated when non-zero (i.e. only adjust the documentation part here).

Jan


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