[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 2/4] iommu VT-d: separate rmrr addition function
>>> On 29.05.15 at 23:38, <elena.ufimtseva@xxxxxxxxxx> wrote: > 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. Certainly there is. (And the statement wouldn't belong here anyway, but below the first --- separator.) > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> And certainly I didn't approve it in this shape: > +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; You _again_ made this an error, which it wasn't before. A little more care please. Also you folded the leak fix into here without saying so. As said on the solitary leak fix patch - that change belongs there (not the least because we will want to backport that but not this one). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |