Re: [RFC v1 5/5] xen/arm: add SCI mediator support for DomUs

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Dec 17, 2021 at 12:15:25PM +0000, Oleksii Moisieiev wrote:
> > On 14.12.21 11:34, Oleksii Moisieiev wrote:
> > > @@ -1817,17 +1858,24 @@ static void libxl__add_dtdevs(libxl__egc *egc, 
> > > libxl__ao *ao, uint32_t domid,
> > >   {
> > >       AO_GC;
> > >       libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
> > > -    int i, rc = 0;
> > > +    int i, rc = 0, rc_sci = 0;
> > >       for (i = 0; i < d_config->num_dtdevs; i++) {
> > >           const libxl_device_dtdev *dtdev = &d_config->dtdevs[i];
> > >           LOGD(DEBUG, domid, "Assign device \"%s\" to domain", 
> > > dtdev->path);
> > >           rc = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
> > > -        if (rc < 0) {
> > > -            LOGD(ERROR, domid, "xc_assign_dtdevice failed: %d", rc);
> > > +        rc_sci = xc_domain_add_sci_device(CTX->xch, domid, dtdev->path);
> > > +
> > > +        if ((rc < 0) && (rc_sci < 0)) {
> > > +            LOGD(ERROR, domid, "xc_assign_dt_device failed: %d; "
> > > +                 "xc_domain_add_sci_device failed: %d",
> > > +                 rc, rc_sci);
> > >               goto out;
> > >           }
> > > +
> > > +        if (rc)
> > > +            rc = rc_sci;
> > 
> > 
> > If I get this code right, it sounds like the dom.cfg's dtdev property is
> > reused to describe sci devices as well, but the libxl__add_dtdevs() does not
> > (and can not) differentiate them. So it has no option but to send two
> > domctls for each device in dtdevs[] hoping to at least one domctl to
> > succeeded. Or I really missed something?
> > 
> > It feels to me that:
> >  - either new dom.cfg's property could be introduced (scidev?) to describe
> > sci devices together with new parsing logic/management code, so you will end
> > up having new libxl__add_scidevs() to invoke XEN_DOMCTL_add_sci_device,
> > so no mixing things.
> >  - or indeed dtdev logic could be *completely* reused including extending
> > XEN_DOMCTL_assign_device to cover your use-case, although sounds generic, it
> > is used to describe devices for the passthrough (to configure an IOMMU for
> > the device), in that case you wouldn't need an extra
> > XEN_DOMCTL_add_sci_device introduced by current patch.
> > 
> > Personally I would use the first option as I am not sure that second option
> > is conceptually correct && possible. I would leave this for the maintainers
> > to clarify.
> > 
> 
> Thank you for the point. I agree that reusing XEN_DOMCTL_assign_device
> seems not to be conceptually correct. Introducing new dom.cfg property
> seems to be the only way to avoid extra Domctl calls.
> I will handle this in v2 if there will be no complains from Maintainers.

I don't know if there will be a complains in v2 or not :-), but using
something different from dtdev sound good.

If I understand correctly, dtdev seems to be about passing through an
existing device to a guest, and this new sci device seems to be about having Xen
"emulating" an sci device which the guest will use. So introducing
something new (scidev or other name) sounds good.

Thanks,

-- 
Anthony PERARD