Re: [Xen-devel] [PATCH v6 2/4] iommu VT-d: separate rmrr addition function

[Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>]

On Mon, Jun 01, 2015 at 04:51:55AM +0000, Tian, Kevin wrote:
> > From: elena.ufimtseva@xxxxxxxxxx [mailto:elena.ufimtseva@xxxxxxxxxx]
> > Sent: Saturday, May 30, 2015 5:39 AM
> > 
> > From: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>
> > 
> > In preparation for auxiliary RMRR data provided on Xen
> > command line, make RMRR adding a separate function.
> > Also free memery for rmrr device scope in error path.
> > No changes since v5.
> > 
> > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> >  xen/drivers/passthrough/vtd/dmar.c | 129
> > ++++++++++++++++++++-----------------
> >  1 file changed, 70 insertions(+), 59 deletions(-)
> > 
> > diff --git a/xen/drivers/passthrough/vtd/dmar.c
> > b/xen/drivers/passthrough/vtd/dmar.c
> > index 0985150..89a2f79 100644
> > --- a/xen/drivers/passthrough/vtd/dmar.c
> > +++ b/xen/drivers/passthrough/vtd/dmar.c
> > @@ -576,6 +576,73 @@ out:
> >      return ret;
> >  }
> > 
> > +static int register_one_rmrr(struct acpi_rmrr_unit *rmrru)
> > +{
> > +    bool_t ignore = 0;
> > +    unsigned int i = 0;
> > +    int ret = 0;
> > +
> > +    /* Skip checking if segment is not accessible yet. */
> > +    if ( !pci_known_segment(rmrru->segment) )
> > +    {
> > +        dprintk(XENLOG_WARNING VTDPREFIX, "UNKNOWN Prefix! %04x",
> > rmrru->segment);
> > +        i = UINT_MAX;
> > +    }
> > +
> > +    for ( ; i < rmrru->scope.devices_cnt; i++ )
> > +    {
> > +        u8 b = PCI_BUS(rmrru->scope.devices[i]);
> > +        u8 d = PCI_SLOT(rmrru->scope.devices[i]);
> > +        u8 f = PCI_FUNC(rmrru->scope.devices[i]);
> > +
> > +        if ( pci_device_detect(rmrru->segment, b, d, f) == 0 )
> > +        {
> > +            dprintk(XENLOG_WARNING VTDPREFIX,
> > +                    " Non-existent device (%04x:%02x:%02x.%u) is
> > reported"
> > +                    " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n",
> > +                    rmrru->segment, b, d, f,
> > +                    rmrru->base_address, rmrru->end_address);
> > +            ignore = 1;
> > +        }
> > +        else
> > +        {
> > +            ignore = 0;
> > +            break;
> > +        }
> > +    }
> > +
> > +    if ( ignore )
> > +    {
> > +        dprintk(XENLOG_WARNING VTDPREFIX,
> > +            "  Ignore the RMRR (%"PRIx64", %"PRIx64") due to "
> > +            "devices under its scope are not PCI discoverable!\n",
> > +            rmrru->base_address, rmrru->end_address);
> > +        xfree(rmrru->scope.devices);
> > +        xfree(rmrru);
> > +        ret = -EFAULT;
> > +    }
> > +    else if ( rmrru->base_address > rmrru->end_address )
> > +    {
> > +        dprintk(XENLOG_WARNING VTDPREFIX,
> > +            "  The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n",
> > +            rmrru->base_address, rmrru->end_address);
> > +        xfree(rmrru->scope.devices);
> > +        xfree(rmrru);
> > +        ret = -EFAULT;
> > +    }
> 
> above two error handling can be combined into one at the end of the
> func like in other places.
> 
> Thanks
> Kevin

Hi Kevin

Thank you for review.
I think in this case I cannot combine these two as the ret should not be
set in first (ignore) branch. Looks like I placed it there by mistake.

Elena

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