Re: [Xen-devel] [v5][PATCH 10/16] tools: introduce some new parameters to set rdm policy

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

> > Global RDM parameter, "type", allows user to specify reserved regions
> > explicitly, e.g. using 'host' to include all reserved regions reported
> As I understand this feature, I don't think the name `type' is right.
> This is a method for handling or overriding/ignoring the RDM problem.
>
> Perhaps `handling=' or `strategy=' ?
Okay, I prefer to use 'strategy='.

> And `none' seems quite an innocuous name for such a dangerous
> setting.  How about `ignore' ?
>
> > 'strict/relaxed' policy decides how to handle conflict when reserving RDM
> > regions in pfn space. If conflict exists, 'strict' means an immediate error
> > so VM can't keep running, while 'relaxed' allows moving forward with a
> > warning message thrown out.
> >
> > Default per-device RDM policy is 'strict', while default global RDM policy
> > is 'relaxed'. And the per-device policy would override the global policy like
> > others.
> I don't think these two sentences can both be true.  If the per-device
> policy overrides the global policy, then the per-device policy must
> have at least three values: `strict', `relaxed', and `not set'
> (implicitly using global policy).
Yeah. The per-device policy is first set as the global policy. This 
behavior is same as other pci option. Please see the last patch,
@@ -1937,6 +1945,11 @@ skip_vfb:
             pcidev->power_mgmt = pci_power_mgmt;
             pcidev->permissive = pci_permissive;
             pcidev->seize = pci_seize;
+            /*
+             * Like other pci option, the per-device policy always follows
+             * the global policy by default.
+             */
+            pcidev->rdm_reserve = b_info->u.hvm.rdm.reserve;
             if (!xlu_pci_parse_bdf(config, pcidev, buf))
                 d_config->num_pcidevs++;
         }

> Perhaps I have misunderstood.
>
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index a3e0e2e..a2dc343 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -655,6 +655,83 @@ assigned slave device.
> ...
> > +=item B<type="STRING">
> The documentation on this is rather vague about what the default is.
> But you do say:
>
> > +In this conflict case,
> > +
> > +#1. If the type options is set with "none",
> > +
> > +rdm = "type=none,reserve=strict" or rdm = "type=none,reserve=relaxed"
> > +
> > +mean we don't handle any conflict just to make VM keep running as before.
> > +Note this is our default behavior.
> This suggests that the default is "do the dangerous thing".  That
> doesn't seem right.
As I discussed with Campbell we'd like not to expose "none" in xl level 
since this is equivalent to that case we don't set anything.
> > +#2. If the type options is set with "host",
> > +
> > +rdm = "type=host,reserve=strict" or rdm = "type=host,reserve=relaxed"
> ...
> > +"relaxed" allows a VM to be created to keep running with a warning message
> > +thrown out. But this may crash this VM if this device accesses RDM. For 
> > example,
> > +Windows IGD GFX driver always access these regions so this lead to a blue 
> > screen
> > +to crash VM in such a case.
> The difference between
>     type=host,reserve=relaxed
> and
>     type=none
> is simply whether a warning is printed ?
"none" means we have a chance to work as before since not all devices 
own RDM. But as I said above this is same as !rdm.
Thanks
Tiejun

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