Re: [Xen-devel] [RFC][PATCH 07/13] xen/passthrough: extend hypercall to support rdm reservation policy

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.04.15 at 14:32, <tiejun.chen@xxxxxxxxx> wrote:
> On 2015/4/16 23:40, Tim Deegan wrote:
>> At 17:21 +0800 on 10 Apr (1428686518), Tiejun Chen wrote:
>>> @@ -1851,7 +1857,14 @@ static int rmrr_identity_mapping(struct domain *d, 
>>> bool_t map,
>>>           if ( !is_hardware_domain(d) )
>>>           {
>>>               if ( (err = set_identity_p2m_entry(d, base_pfn, 
>>> p2m_access_rw)) )
>>> -                return err;
>>> +            {
>>> +                if ( flag == XEN_DOMCTL_PCIDEV_RDM_TRY )
>>> +                {
>>> +                    printk(XENLOG_G_WARNING "Some devices may work failed 
>>> .\n");
>>
>> This is a bit cryptic.  How about:
>>   "RMRR map failed.  Device %04x:%02x:%02x.%u and domain %d may be 
> unstable.",
>> (and pass in the devfn from the caller so we can print the details of
>> the device).
> 
> Got it but we can't get SBDF here directly.
> 
> So just now we can have this line.
> 
>          {
>              if ( flag == XEN_DOMCTL_PCIDEV_RDM_TRY )
>                  dprintk(XENLOG_ERR VTDPREFIX,
>                          "RMRR mapping failed to pfn:%"PRIx64""
>                          " so Dom%d may be unstable.\n",
>                          base_pfn, d->domain_id);
>              else
>                  return err;
>          }
> 
> Certainly, we can extend rmrr_identity_mapping() to own its associated 
> SBDF as an input parameter (and bring some syncs) if you still think 
> this is necessary.

I don't think we can, since a single RMRR may be associated with
more than one device.

>>> @@ -493,6 +493,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_sendtrigger_t);
>>>   /* XEN_DOMCTL_deassign_device */
>>>   struct xen_domctl_assign_device {
>>>       uint32_t  machine_sbdf;   /* machine PCI ID of assigned device */
>>> +    /* IN */
>>> +#define XEN_DOMCTL_PCIDEV_RDM_TRY       0
>>> +#define XEN_DOMCTL_PCIDEV_RDM_FORCE     1
>>
>> "STRICT" might be a better word than "FORCE" (here and everywhere
>> else).  "FORCE" sounds like either Xen will assign the device even if
>> it's unsafe,  which is the opposite of what's meant IIUC.
> 
> This is definitely fine to me but this is derived from our policy based 
> on that previous design,
> 
> Global RDM parameter:
>      rdm = [ 'host, reserve=force/try' ]
> Per-device RDM parameter:
>      pci = [ 'sbdf, rdm_reserve=force/try' ]
> 
> Please refer to patch #1. So I guess we need a further agreement or 
> comments from other guys :)

I think I'd prefer strict/relaxed over force/try, but I certainly can
live with the latter.

Jan


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