1: check all of an RMRR for being E820-reserved 2: adjust logging of RMRRs Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Checking just the first and last page is not sufficient (and redundant for single-page regions). As we don't need to care about IA64 anymore, use an x86-specific function to get this done without looping over each individual page. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -29,6 +29,7 @@ #include <xen/pci.h> #include <xen/pci_regs.h> #include <asm/atomic.h> +#include <asm/e820.h> #include <asm/string.h> #include "dmar.h" #include "iommu.h" @@ -632,14 +633,11 @@ acpi_parse_one_rmrr(struct acpi_dmar_hea * not properly represented in the system memory map and * inform the user */ - if ( (!page_is_ram_type(paddr_to_pfn(base_addr), RAM_TYPE_RESERVED)) || - (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) ) - { + if ( !e820_all_mapped(base_addr, end_addr + 1, RAM_TYPE_RESERVED) ) printk(XENLOG_WARNING VTDPREFIX " RMRR address range %"PRIx64"..%"PRIx64" not in reserved memory;" " need \"iommu_inclusive_mapping=1\"?\n", base_addr, end_addr); - } rmrru = xzalloc(struct acpi_rmrr_unit); if ( !rmrru ) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich <jbeulich@suse.com> > Sent: Thursday, February 6, 2020 9:31 PM > > Checking just the first and last page is not sufficient (and redundant > for single-page regions). As we don't need to care about IA64 anymore, > use an x86-specific function to get this done without looping over each > individual page. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On Thu, Feb 06, 2020 at 02:31:03PM +0100, Jan Beulich wrote: > Checking just the first and last page is not sufficient (and redundant > for single-page regions). As we don't need to care about IA64 anymore, > use an x86-specific function to get this done without looping over each > individual page. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/drivers/passthrough/vtd/dmar.c > +++ b/xen/drivers/passthrough/vtd/dmar.c > @@ -29,6 +29,7 @@ > #include <xen/pci.h> > #include <xen/pci_regs.h> > #include <asm/atomic.h> > +#include <asm/e820.h> > #include <asm/string.h> > #include "dmar.h" > #include "iommu.h" > @@ -632,14 +633,11 @@ acpi_parse_one_rmrr(struct acpi_dmar_hea > * not properly represented in the system memory map and > * inform the user > */ > - if ( (!page_is_ram_type(paddr_to_pfn(base_addr), RAM_TYPE_RESERVED)) || > - (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) ) > - { > + if ( !e820_all_mapped(base_addr, end_addr + 1, RAM_TYPE_RESERVED) ) Do you need to add one to the end? The other user of e820_all_mapped seems to treat end as start + size - 1, which makes me think the parameters to the function are an inclusive range [start, end] and that's what's present in the RMRR ACPI entries? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 07.02.2020 13:20, Roger Pau Monné wrote: > On Thu, Feb 06, 2020 at 02:31:03PM +0100, Jan Beulich wrote: >> Checking just the first and last page is not sufficient (and redundant >> for single-page regions). As we don't need to care about IA64 anymore, >> use an x86-specific function to get this done without looping over each >> individual page. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/drivers/passthrough/vtd/dmar.c >> +++ b/xen/drivers/passthrough/vtd/dmar.c >> @@ -29,6 +29,7 @@ >> #include <xen/pci.h> >> #include <xen/pci_regs.h> >> #include <asm/atomic.h> >> +#include <asm/e820.h> >> #include <asm/string.h> >> #include "dmar.h" >> #include "iommu.h" >> @@ -632,14 +633,11 @@ acpi_parse_one_rmrr(struct acpi_dmar_hea >> * not properly represented in the system memory map and >> * inform the user >> */ >> - if ( (!page_is_ram_type(paddr_to_pfn(base_addr), RAM_TYPE_RESERVED)) || >> - (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) ) >> - { >> + if ( !e820_all_mapped(base_addr, end_addr + 1, RAM_TYPE_RESERVED) ) > > Do you need to add one to the end? > > The other user of e820_all_mapped seems to treat end as start + size > - 1, which makes me think the parameters to the function are an > inclusive range [start, end] and that's what's present in the RMRR > ACPI entries? Well, it's the implementation of the function which matters. This one other caller is wrong afaict, and I've just sent a patch. The non-inclusiveness is also in line with Linux'es variant of the function (where we've got ours from originally, just that it has been renamed and slightly extended since then). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri, Feb 07, 2020 at 02:27:11PM +0100, Jan Beulich wrote: > On 07.02.2020 13:20, Roger Pau Monné wrote: > > On Thu, Feb 06, 2020 at 02:31:03PM +0100, Jan Beulich wrote: > >> Checking just the first and last page is not sufficient (and redundant > >> for single-page regions). As we don't need to care about IA64 anymore, > >> use an x86-specific function to get this done without looping over each > >> individual page. > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > >> > >> --- a/xen/drivers/passthrough/vtd/dmar.c > >> +++ b/xen/drivers/passthrough/vtd/dmar.c > >> @@ -29,6 +29,7 @@ > >> #include <xen/pci.h> > >> #include <xen/pci_regs.h> > >> #include <asm/atomic.h> > >> +#include <asm/e820.h> > >> #include <asm/string.h> > >> #include "dmar.h" > >> #include "iommu.h" > >> @@ -632,14 +633,11 @@ acpi_parse_one_rmrr(struct acpi_dmar_hea > >> * not properly represented in the system memory map and > >> * inform the user > >> */ > >> - if ( (!page_is_ram_type(paddr_to_pfn(base_addr), RAM_TYPE_RESERVED)) || > >> - (!page_is_ram_type(paddr_to_pfn(end_addr), RAM_TYPE_RESERVED)) ) > >> - { > >> + if ( !e820_all_mapped(base_addr, end_addr + 1, RAM_TYPE_RESERVED) ) > > > > Do you need to add one to the end? > > > > The other user of e820_all_mapped seems to treat end as start + size > > - 1, which makes me think the parameters to the function are an > > inclusive range [start, end] and that's what's present in the RMRR > > ACPI entries? > > Well, it's the implementation of the function which matters. This > one other caller is wrong afaict, and I've just sent a patch. The > non-inclusiveness is also in line with Linux'es variant of the > function (where we've got ours from originally, just that it has > been renamed and slightly extended since then). Thanks for fixing that other caller and clarifying the comment. Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Consistently use [,] range representation, shrink leading double blanks to a single one, and slightly adjust text in some cases. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -561,7 +561,7 @@ static int register_one_rmrr(struct acpi { dprintk(XENLOG_WARNING VTDPREFIX, " Non-existent device (%04x:%02x:%02x.%u) is reported" - " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n", + " in RMRR [%"PRIx64",%"PRIx64")]'s scope!\n", rmrru->segment, b, d, f, rmrru->base_address, rmrru->end_address); ignore = true; @@ -577,8 +577,8 @@ static int register_one_rmrr(struct acpi if ( ignore ) { dprintk(XENLOG_WARNING VTDPREFIX, - " Ignore the RMRR (%"PRIx64", %"PRIx64") due to " - "devices under its scope are not PCI discoverable!\n", + " Ignore RMRR [%"PRIx64",%"PRIx64"] as no device" + " under its scope is PCI discoverable!\n", rmrru->base_address, rmrru->end_address); scope_devices_free(&rmrru->scope); xfree(rmrru); @@ -586,7 +586,7 @@ static int register_one_rmrr(struct acpi else if ( rmrru->base_address > rmrru->end_address ) { dprintk(XENLOG_WARNING VTDPREFIX, - " The RMRR (%"PRIx64", %"PRIx64") is incorrect!\n", + " RMRR [%"PRIx64",%"PRIx64"] is incorrect!\n", rmrru->base_address, rmrru->end_address); scope_devices_free(&rmrru->scope); xfree(rmrru); @@ -595,8 +595,7 @@ static int register_one_rmrr(struct acpi else { if ( iommu_verbose ) - dprintk(VTDPREFIX, - " RMRR region: base_addr %"PRIx64" end_addr %"PRIx64"\n", + dprintk(VTDPREFIX, " RMRR: [%"PRIx64",%"PRIx64"]\n", rmrru->base_address, rmrru->end_address); acpi_register_rmrr_unit(rmrru); } @@ -635,7 +634,7 @@ acpi_parse_one_rmrr(struct acpi_dmar_hea */ if ( !e820_all_mapped(base_addr, end_addr + 1, RAM_TYPE_RESERVED) ) printk(XENLOG_WARNING VTDPREFIX - " RMRR address range %"PRIx64"..%"PRIx64" not in reserved memory;" + " RMRR [%"PRIx64",%"PRIx64"] not in reserved memory;" " need \"iommu_inclusive_mapping=1\"?\n", base_addr, end_addr); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> From: Jan Beulich <jbeulich@suse.com> > Sent: Thursday, February 6, 2020 9:31 PM > To: xen-devel@lists.xenproject.org > Cc: Tian, Kevin <kevin.tian@intel.com> > Subject: [PATCH 2/2] VT-d: adjust logging of RMRRs > > Consistently use [,] range representation, shrink leading double blanks > to a single one, and slightly adjust text in some cases. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On Thu, Feb 6, 2020 at 8:33 AM Jan Beulich <jbeulich@suse.com> wrote: > > Consistently use [,] range representation, shrink leading double blanks > to a single one, and slightly adjust text in some cases. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/drivers/passthrough/vtd/dmar.c > +++ b/xen/drivers/passthrough/vtd/dmar.c > @@ -561,7 +561,7 @@ static int register_one_rmrr(struct acpi > { > dprintk(XENLOG_WARNING VTDPREFIX, > " Non-existent device (%04x:%02x:%02x.%u) is reported" > - " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n", > + " in RMRR [%"PRIx64",%"PRIx64")]'s scope!\n", Missed removing the ")". With that fixed, Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Regards, Jason _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 06.02.2020 19:46, Jason Andryuk wrote: > On Thu, Feb 6, 2020 at 8:33 AM Jan Beulich <jbeulich@suse.com> wrote: >> >> Consistently use [,] range representation, shrink leading double blanks >> to a single one, and slightly adjust text in some cases. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/drivers/passthrough/vtd/dmar.c >> +++ b/xen/drivers/passthrough/vtd/dmar.c >> @@ -561,7 +561,7 @@ static int register_one_rmrr(struct acpi >> { >> dprintk(XENLOG_WARNING VTDPREFIX, >> " Non-existent device (%04x:%02x:%02x.%u) is reported" >> - " in RMRR (%"PRIx64", %"PRIx64")'s scope!\n", >> + " in RMRR [%"PRIx64",%"PRIx64")]'s scope!\n", > > Missed removing the ")". > > With that fixed, Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Oh, indeed - thanks for noticing! Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.