... by a config option, selected only from privileged Xen configurations.
Also eliminate the pointless new macro ROUND_UP_TO_PAGESIZE().
Finally, I''m not really understanding the need for two command line
options (reassigndev= and reassign_resources) here - wouldn''t the
former suffice? Specifying one without the other doesn''t seem to make
much sense...
As usual, written and tested on 2.6.27 and made apply to the 2.6.18
tree without further testing.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Index: head-2008-10-13/drivers/pci/Kconfig
==================================================================---
head-2008-10-13.orig/drivers/pci/Kconfig 2008-10-13 15:11:33.000000000 +0200
+++ head-2008-10-13/drivers/pci/Kconfig 2008-10-13 15:14:53.000000000 +0200
@@ -21,6 +21,9 @@ config PCI_MSI
If you don''t know what to do here, say N.
+config PCI_REASSIGN
+ bool
+
config PCI_DEBUG
bool "PCI Debugging"
depends on PCI && DEBUG_KERNEL
Index: head-2008-10-13/drivers/pci/Makefile
==================================================================---
head-2008-10-13.orig/drivers/pci/Makefile 2008-10-13 15:11:33.000000000 +0200
+++ head-2008-10-13/drivers/pci/Makefile 2008-10-13 15:14:53.000000000 +0200
@@ -3,8 +3,8 @@
#
obj-y += access.o bus.o probe.o remove.o pci.o quirks.o \
- pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
- reassigndev.o
+ pci-driver.o search.o pci-sysfs.o rom.o setup-res.o
+obj-$(CONFIG_PCI_REASSIGN) += reassigndev.o
obj-$(CONFIG_PROC_FS) += proc.o
# Build PCI Express stuff if needed
Index: head-2008-10-13/drivers/pci/pci.h
==================================================================---
head-2008-10-13.orig/drivers/pci/pci.h 2008-10-13 15:11:33.000000000 +0200
+++ head-2008-10-13/drivers/pci/pci.h 2008-10-13 15:14:53.000000000 +0200
@@ -144,8 +144,11 @@ struct pci_slot_attribute {
return NULL;
}
-#define ROUND_UP_TO_PAGESIZE(size) ((size + PAGE_SIZE - 1) & ~(PAGE_SIZE -
1))
-
+#ifdef CONFIG_PCI_REASSIGN
extern int reassign_resources;
extern int is_reassigndev(struct pci_dev *dev);
extern void pci_update_bridge(struct pci_dev *dev, int resno);
+#else
+#define reassign_resources 0
+#define is_reassigndev(dev) 0
+#endif
Index: head-2008-10-13/drivers/pci/setup-bus.c
==================================================================---
head-2008-10-13.orig/drivers/pci/setup-bus.c 2008-10-13 15:11:33.000000000 +0200
+++ head-2008-10-13/drivers/pci/setup-bus.c 2008-10-13 15:14:53.000000000 +0200
@@ -354,9 +354,8 @@ static int pbus_size_mem(struct pci_bus
continue;
r_size = r->end - r->start + 1;
- if (reassign) {
- r_size = ROUND_UP_TO_PAGESIZE(r_size);
- }
+ if (reassign)
+ r_size = ALIGN(r_size, PAGE_SIZE);
/* For bridges size != alignment */
align = (i < PCI_BRIDGE_RESOURCES) ? r_size : r->start;
Index: head-2008-10-13/drivers/pci/setup-res.c
==================================================================---
head-2008-10-13.orig/drivers/pci/setup-res.c 2008-10-13 15:11:33.000000000 +0200
+++ head-2008-10-13/drivers/pci/setup-res.c 2008-10-13 15:14:53.000000000 +0200
@@ -126,7 +126,8 @@ pci_claim_resource(struct pci_dev *dev,
}
EXPORT_SYMBOL_GPL(pci_claim_resource);
-void
+#ifdef CONFIG_PCI_REASSIGN
+void
pci_update_bridge(struct pci_dev *dev, int resno)
{
struct resource *res = &dev->resource[resno];
@@ -193,6 +194,7 @@ pci_update_bridge(struct pci_dev *dev, i
break;
}
}
+#endif
int pci_assign_resource(struct pci_dev *dev, int resno)
{
@@ -215,7 +217,7 @@ int pci_assign_resource(struct pci_dev *
align = size;
if ((reassigndev) &&
(res->flags & IORESOURCE_MEM)) {
- align = ROUND_UP_TO_PAGESIZE(align);
+ align = ALIGN(align, PAGE_SIZE);
}
} else {
align = res->start;
@@ -242,9 +244,11 @@ int pci_assign_resource(struct pci_dev *
resno, (unsigned long long)size,
(unsigned long long)res->start, pci_name(dev));
} else if (resno < PCI_BRIDGE_RESOURCES) {
- printk(KERN_DEBUG "PCI: Assign resource(%d) on %s "
- "%016llx - %016llx\n", resno, pci_name(dev),
- (u64)res->start, (u64)res->end);
+ if (reassign_resources && is_reassigndev(dev))
+ printk(KERN_DEBUG "PCI: Assign resource(%d) on %s "
+ "%016llx - %016llx\n", resno, pci_name(dev),
+ (unsigned long long)res->start,
+ (unsigned long long)res->end);
pci_update_resource(dev, res, resno);
}
Index: head-2008-10-13/drivers/xen/Kconfig
==================================================================---
head-2008-10-13.orig/drivers/xen/Kconfig 2008-10-13 15:14:34.000000000 +0200
+++ head-2008-10-13/drivers/xen/Kconfig 2008-10-13 15:16:22.000000000 +0200
@@ -16,6 +16,7 @@ menu "XEN"
config XEN_PRIVILEGED_GUEST
bool "Privileged Guest (domain 0)"
+ select PCI_REASSIGN if PCI
help
Support for privileged operation (domain 0)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Yuji Shimada
2008-Oct-15 07:56 UTC
Re: [Xen-devel] [PATCH] conditionalize PCI reassign code
On Tue, 14 Oct 2008 14:39:50 +0100 "Jan Beulich" <jbeulich@novell.com> wrote:> Finally, I''m not really understanding the need for two command line > options (reassigndev= and reassign_resources) here - wouldn''t the > former suffice? Specifying one without the other doesn''t seem to make > much sense...I agree with you that reassign_resources option is not needed. I will submit the patch to remove it, after submitting the patch to fix the issue which was found by Zhao, Yu. Thanks, -- Yuji Shimada _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hello Yuji, It would be great if you could submit your patch to the linux-pci. Since the native kernel (KVM) may also interested in this feature. Thanks, Yu Yuji Shimada wrote:> On Tue, 14 Oct 2008 14:39:50 +0100 > "Jan Beulich" <jbeulich@novell.com> wrote: > >> Finally, I''m not really understanding the need for two command line >> options (reassigndev= and reassign_resources) here - wouldn''t the >> former suffice? Specifying one without the other doesn''t seem to make >> much sense... > > I agree with you that reassign_resources option is not needed. > > I will submit the patch to remove it, after submitting the patch to > fix the issue which was found by Zhao, Yu. > > Thanks, > > -- > Yuji Shimada > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
There is build error (c/s 701) with this patch, please take a look. Thanks. Jan Beulich wrote:> ... by a config option, selected only from privileged Xen configurations. > > Also eliminate the pointless new macro ROUND_UP_TO_PAGESIZE(). > > Finally, I''m not really understanding the need for two command line > options (reassigndev= and reassign_resources) here - wouldn''t the > former suffice? Specifying one without the other doesn''t seem to make > much sense... > > As usual, written and tested on 2.6.27 and made apply to the 2.6.18 > tree without further testing. > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > > Index: head-2008-10-13/drivers/pci/Kconfig > ==================================================================> --- head-2008-10-13.orig/drivers/pci/Kconfig 2008-10-13 15:11:33.000000000 +0200 > +++ head-2008-10-13/drivers/pci/Kconfig 2008-10-13 15:14:53.000000000 +0200 > @@ -21,6 +21,9 @@ config PCI_MSI > > If you don''t know what to do here, say N. > > +config PCI_REASSIGN > + bool > + > config PCI_DEBUG > bool "PCI Debugging" > depends on PCI && DEBUG_KERNEL > Index: head-2008-10-13/drivers/pci/Makefile > ==================================================================> --- head-2008-10-13.orig/drivers/pci/Makefile 2008-10-13 15:11:33.000000000 +0200 > +++ head-2008-10-13/drivers/pci/Makefile 2008-10-13 15:14:53.000000000 +0200 > @@ -3,8 +3,8 @@ > # > > obj-y += access.o bus.o probe.o remove.o pci.o quirks.o \ > - pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \ > - reassigndev.o > + pci-driver.o search.o pci-sysfs.o rom.o setup-res.o > +obj-$(CONFIG_PCI_REASSIGN) += reassigndev.o > obj-$(CONFIG_PROC_FS) += proc.o > > # Build PCI Express stuff if needed > Index: head-2008-10-13/drivers/pci/pci.h > ==================================================================> --- head-2008-10-13.orig/drivers/pci/pci.h 2008-10-13 15:11:33.000000000 +0200 > +++ head-2008-10-13/drivers/pci/pci.h 2008-10-13 15:14:53.000000000 +0200 > @@ -144,8 +144,11 @@ struct pci_slot_attribute { > return NULL; > } > > -#define ROUND_UP_TO_PAGESIZE(size) ((size + PAGE_SIZE - 1) & ~(PAGE_SIZE - 1)) > - > +#ifdef CONFIG_PCI_REASSIGN > extern int reassign_resources; > extern int is_reassigndev(struct pci_dev *dev); > extern void pci_update_bridge(struct pci_dev *dev, int resno); > +#else > +#define reassign_resources 0 > +#define is_reassigndev(dev) 0 > +#endif > Index: head-2008-10-13/drivers/pci/setup-bus.c > ==================================================================> --- head-2008-10-13.orig/drivers/pci/setup-bus.c 2008-10-13 15:11:33.000000000 +0200 > +++ head-2008-10-13/drivers/pci/setup-bus.c 2008-10-13 15:14:53.000000000 +0200 > @@ -354,9 +354,8 @@ static int pbus_size_mem(struct pci_bus > continue; > r_size = r->end - r->start + 1; > > - if (reassign) { > - r_size = ROUND_UP_TO_PAGESIZE(r_size); > - } > + if (reassign) > + r_size = ALIGN(r_size, PAGE_SIZE); > > /* For bridges size != alignment */ > align = (i < PCI_BRIDGE_RESOURCES) ? r_size : r->start; > Index: head-2008-10-13/drivers/pci/setup-res.c > ==================================================================> --- head-2008-10-13.orig/drivers/pci/setup-res.c 2008-10-13 15:11:33.000000000 +0200 > +++ head-2008-10-13/drivers/pci/setup-res.c 2008-10-13 15:14:53.000000000 +0200 > @@ -126,7 +126,8 @@ pci_claim_resource(struct pci_dev *dev, > } > EXPORT_SYMBOL_GPL(pci_claim_resource); > > -void > +#ifdef CONFIG_PCI_REASSIGN > +void > pci_update_bridge(struct pci_dev *dev, int resno) > { > struct resource *res = &dev->resource[resno]; > @@ -193,6 +194,7 @@ pci_update_bridge(struct pci_dev *dev, i > break; > } > } > +#endif > > int pci_assign_resource(struct pci_dev *dev, int resno) > { > @@ -215,7 +217,7 @@ int pci_assign_resource(struct pci_dev * > align = size; > if ((reassigndev) && > (res->flags & IORESOURCE_MEM)) { > - align = ROUND_UP_TO_PAGESIZE(align); > + align = ALIGN(align, PAGE_SIZE); > } > } else { > align = res->start; > @@ -242,9 +244,11 @@ int pci_assign_resource(struct pci_dev * > resno, (unsigned long long)size, > (unsigned long long)res->start, pci_name(dev)); > } else if (resno < PCI_BRIDGE_RESOURCES) { > - printk(KERN_DEBUG "PCI: Assign resource(%d) on %s " > - "%016llx - %016llx\n", resno, pci_name(dev), > - (u64)res->start, (u64)res->end); > + if (reassign_resources && is_reassigndev(dev)) > + printk(KERN_DEBUG "PCI: Assign resource(%d) on %s " > + "%016llx - %016llx\n", resno, pci_name(dev), > + (unsigned long long)res->start, > + (unsigned long long)res->end); > pci_update_resource(dev, res, resno); > } > > Index: head-2008-10-13/drivers/xen/Kconfig > ==================================================================> --- head-2008-10-13.orig/drivers/xen/Kconfig 2008-10-13 15:14:34.000000000 +0200 > +++ head-2008-10-13/drivers/xen/Kconfig 2008-10-13 15:16:22.000000000 +0200 > @@ -16,6 +16,7 @@ menu "XEN" > > config XEN_PRIVILEGED_GUEST > bool "Privileged Guest (domain 0)" > + select PCI_REASSIGN if PCI > help > Support for privileged operation (domain 0) > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Oct-17 07:17 UTC
Re: [Xen-devel] [PATCH] conditionalize PCI reassign code
>>> "Zhao, Yu" <yu.zhao@intel.com> 17.10.08 04:43 >>> >There is build error (c/s 701) with this patch, please take a look.This is why I always include>> As usual, written and tested on 2.6.27 and made apply to the 2.6.18 >> tree without further testing.I''m making a best effort attempt at getting things in proper state for that tree, but time-wise can''t afford full validation. But I always assumed that if this appears in the real (not the staging) tree, it at least survived building in Citrix'' trees - is that not correct, Keir? As you observed the build error - why don''t you provide at least the error the compiler generated? That might make it possible to provide a fix quickly. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Oct-17 07:22 UTC
Re: [Xen-devel] [PATCH] conditionalize PCI reassign code
On 17/10/08 08:17, "Jan Beulich" <jbeulich@novell.com> wrote:>>> As usual, written and tested on 2.6.27 and made apply to the 2.6.18 >>> tree without further testing. > > I''m making a best effort attempt at getting things in proper state for that > tree, but time-wise can''t afford full validation. But I always assumed that > if this appears in the real (not the staging) tree, it at least survived > building in Citrix'' trees - is that not correct, Keir?Yes, but obviously only one configuration is tested. If this build failure is with CONFIG_PCI_REASSIGN we would not pick that up unless it''s in the default buildconfig. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich wrote:>>>> "Zhao, Yu" <yu.zhao@intel.com> 17.10.08 04:43 >>> >> There is build error (c/s 701) with this patch, please take a look. > > This is why I always include > >>> As usual, written and tested on 2.6.27 and made apply to the 2.6.18 >>> tree without further testing. > > I''m making a best effort attempt at getting things in proper state for that > tree, but time-wise can''t afford full validation. But I always assumed that > if this appears in the real (not the staging) tree, it at least survived > building in Citrix'' trees - is that not correct, Keir? > > As you observed the build error - why don''t you provide at least the > error the compiler generated? That might make it possible to provide a > fix quickly.+#ifdef CONFIG_PCI_REASSIGN extern int reassign_resources; extern int is_reassigndev(struct pci_dev *dev); extern void pci_update_bridge(struct pci_dev *dev, int resno); +#else +#define reassign_resources 0 +#define is_reassigndev(dev) 0 +#endif When the CONFIG_PCI_REASSIGN is not set, e.g., a domU kernel, the following line fails to be compiled (drivers/pci/quirks.c): int reassign_resources = 0;> > Jan >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Oct-17 07:27 UTC
Re: [Xen-devel] [PATCH] conditionalize PCI reassign code
>>> Keir Fraser <keir.fraser@eu.citrix.com> 17.10.08 09:22 >>> >On 17/10/08 08:17, "Jan Beulich" <jbeulich@novell.com> wrote: >>>> As usual, written and tested on 2.6.27 and made apply to the 2.6.18 >>>> tree without further testing. >> >> I''m making a best effort attempt at getting things in proper state for that >> tree, but time-wise can''t afford full validation. But I always assumed that >> if this appears in the real (not the staging) tree, it at least survived >> building in Citrix'' trees - is that not correct, Keir? > >Yes, but obviously only one configuration is tested. If this build failure >is with CONFIG_PCI_REASSIGN we would not pick that up unless it''s in the >default buildconfig.CONFIG_PCI_REASSIGN is not a user-visible option, it gets selected from XEN_PRIVILEGED_GUEST. So as long as your builds include a Dom0 kernel build, they''d cover this case. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Oct-17 07:34 UTC
Re: [Xen-devel] [PATCH] conditionalize PCI reassign code
On 17/10/08 08:27, "Zhao, Yu" <yu.zhao@intel.com> wrote:>> As you observed the build error - why don''t you provide at least the >> error the compiler generated? That might make it possible to provide a >> fix quickly. > > +#ifdef CONFIG_PCI_REASSIGN > extern int reassign_resources; > extern int is_reassigndev(struct pci_dev *dev); > extern void pci_update_bridge(struct pci_dev *dev, int resno); > +#else > +#define reassign_resources 0 > +#define is_reassigndev(dev) 0 > +#endif > > When the CONFIG_PCI_REASSIGN is not set, e.g., a domU kernel, the > following line fails to be compiled (drivers/pci/quirks.c): > > int reassign_resources = 0;Ah, the automated tests only build the -xen config. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser wrote:> > > On 17/10/08 08:27, "Zhao, Yu" <yu.zhao@intel.com> wrote: > >>> As you observed the build error - why don''t you provide at least the >>> error the compiler generated? That might make it possible to provide a >>> fix quickly. >> +#ifdef CONFIG_PCI_REASSIGN >> extern int reassign_resources; >> extern int is_reassigndev(struct pci_dev *dev); >> extern void pci_update_bridge(struct pci_dev *dev, int resno); >> +#else >> +#define reassign_resources 0 >> +#define is_reassigndev(dev) 0 >> +#endif >> >> When the CONFIG_PCI_REASSIGN is not set, e.g., a domU kernel, the >> following line fails to be compiled (drivers/pci/quirks.c): >> >> int reassign_resources = 0; > > Ah, the automated tests only build the -xen config. > > -- Keir > >Actually I happened to get it by misusing a native kernel config ... _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Oct-17 08:46 UTC
Re: [Xen-devel] [PATCH] conditionalize PCI reassign code
>>> "Zhao, Yu" <yu.zhao@intel.com> 17.10.08 09:27 >>> >+#ifdef CONFIG_PCI_REASSIGN > extern int reassign_resources; > extern int is_reassigndev(struct pci_dev *dev); > extern void pci_update_bridge(struct pci_dev *dev, int resno); >+#else >+#define reassign_resources 0 >+#define is_reassigndev(dev) 0 >+#endif > >When the CONFIG_PCI_REASSIGN is not set, e.g., a domU kernel, the >following line fails to be compiled (drivers/pci/quirks.c): > >int reassign_resources = 0;Ah, yes, I overlooked that, because when I merged these changes into our kernel the main change to quirks.c needed to be done differently (and then properly in reassigndev.c) anyway, and hence I at once moved the misplaced (and as we agreed elsewhere unnecessary) option into reassigndev.c. So the (untested) patch below should fix it until the patch to eliminate the option will arrive from the original submitter. Signed-off-by: Jan Beulich <jbeulich@novell.com> --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -33,19 +33,6 @@ } __setup("pci-mem-align", set_pci_mem_align); - -int reassign_resources = 0; - -static int __init set_reassign_resources(char *str) -{ - /* resources reassign on */ - reassign_resources = 1; - printk(KERN_DEBUG "PCI: resource reassign ON.\n"); - - return 1; -} -__setup("reassign_resources", set_reassign_resources); - /* This quirk function enables us to force all memory resources which are * assigned to PCI devices, to be page-aligned. */ --- a/drivers/pci/reassigndev.c +++ b/drivers/pci/reassigndev.c @@ -34,6 +34,18 @@ static int pbus_size_mem(struct pci_bus } __setup("reassigndev=", reassigndev_setup); +int reassign_resources = 0; + +static int __init set_reassign_resources(char *str) +{ + /* resources reassign on */ + reassign_resources = 1; + printk(KERN_DEBUG "PCI: resource reassign ON.\n"); + + return 1; +} +__setup("reassign_resources", set_reassign_resources); + int is_reassigndev(struct pci_dev *dev) { char dev_str[TOKEN_MAX+1]; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel