Hollis Blanchard
2007-Jul-06 20:25 UTC
[Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
This is producing warnings for me on PowerPC, which does not define ARCH_HAS_DEV_MEM: /home/hollisb/source/linux-2.6.18-xen-ppc.hg/drivers/xen/char/mem.c:151: warning: ''xen_mmap_mem'' defined but not used /home/hollisb/source/linux-2.6.18-xen-ppc.hg/drivers/xen/char/mem.c:172: warning: ''memory_lseek'' defined but not used /home/hollisb/source/linux-2.6.18-xen-ppc.hg/drivers/xen/char/mem.c:195: warning: ''open_mem'' defined but not used As far as I can see, x86 does not define ARCH_HAS_DEV_MEM either, so should see the same problem? On Fri, 2007-07-06 at 10:15 -0700, Xen patchbot-linux-2.6.18-xen wrote:> # HG changeset patch > # User kfraser@localhost.localdomain > # Date 1183728779 -3600 > # Node ID 4a08141e62ca189577041d8854bb478e5fbe502f > # Parent 9e66b8728bd3a5d857500117c421051ae7dd0d61 > Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations. > Signed-off-by: Jun Kamada <kama@jp.fujitsu.com> > --- > drivers/xen/Kconfig | 4 ++++ > drivers/xen/char/Makefile | 3 +-- > drivers/xen/char/mem.c | 4 ++++ > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff -r 9e66b8728bd3 -r 4a08141e62ca drivers/xen/Kconfig > --- a/drivers/xen/Kconfig Fri Jul 06 14:01:27 2007 +0100 > +++ b/drivers/xen/Kconfig Fri Jul 06 14:32:59 2007 +0100 > @@ -281,4 +281,8 @@ config XEN_XENCOMM > config XEN_XENCOMM > bool > > +config XEN_DEVMEM > + bool > + default y > + > endif > diff -r 9e66b8728bd3 -r 4a08141e62ca drivers/xen/char/Makefile > --- a/drivers/xen/char/Makefile Fri Jul 06 14:01:27 2007 +0100 > +++ b/drivers/xen/char/Makefile Fri Jul 06 14:32:59 2007 +0100 > @@ -1,2 +1,1 @@ > - > -obj-y := mem.o > +obj-$(CONFIG_XEN_DEVMEM) := mem.o > diff -r 9e66b8728bd3 -r 4a08141e62ca drivers/xen/char/mem.c > --- a/drivers/xen/char/mem.c Fri Jul 06 14:01:27 2007 +0100 > +++ b/drivers/xen/char/mem.c Fri Jul 06 14:32:59 2007 +0100 > @@ -33,6 +33,7 @@ static inline int valid_phys_addr_range( > } > #endif > > +#ifdef ARCH_HAS_DEV_MEM > /* > * This funcion reads the *physical* memory. The f_pos points directly to the > * memory location. > @@ -135,6 +136,7 @@ static ssize_t write_mem(struct file * f > *ppos += written; > return written; > } > +#endif > > #ifndef ARCH_HAS_DEV_MEM_MMAP_MEM > static inline int uncached_access(struct file *file) > @@ -194,6 +196,7 @@ static int open_mem(struct inode * inode > return capable(CAP_SYS_RAWIO) ? 0 : -EPERM; > } > > +#ifdef ARCH_HAS_DEV_MEM > const struct file_operations mem_fops = { > .llseek = memory_lseek, > .read = read_mem, > @@ -201,3 +204,4 @@ const struct file_operations mem_fops = > .mmap = xen_mmap_mem, > .open = open_mem, > }; > +#endif > > _______________________________________________ > Xen-changelog mailing list > Xen-changelog@lists.xensource.com > http://lists.xensource.com/xen-changelog-- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jul-07 09:01 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
We define it in asm/mach-xen/asm/io.h. However, the patch *is* of questionable value. It guards the only non-static definition in the file with #ifdef ARCH_HAS_DEV_MEM. Which begs the question why you would build the file at all if !ARCH_HAS_DEV_MEM. I''ll revert it. By the way, I wonder how PPC manages to build both drivers/char/mem.c and drivers/xen/char/mem.c without ARCH_HAS_DEV_MEM? The model is supposed to be that mem_fops defined by the Xen file is picked up by the generic file. If !ARCH_HAS_DEV_MEM then that doesn''t happen -- so who picks up the Xen mem_fops? -- Keir On 6/7/07 21:25, "Hollis Blanchard" <hollisb@us.ibm.com> wrote:> This is producing warnings for me on PowerPC, which does not define > ARCH_HAS_DEV_MEM: > > /home/hollisb/source/linux-2.6.18-xen-ppc.hg/drivers/xen/char/mem.c:151: > warning: ''xen_mmap_mem'' defined but not used > /home/hollisb/source/linux-2.6.18-xen-ppc.hg/drivers/xen/char/mem.c:172: > warning: ''memory_lseek'' defined but not used > /home/hollisb/source/linux-2.6.18-xen-ppc.hg/drivers/xen/char/mem.c:195: > warning: ''open_mem'' defined but not used > > As far as I can see, x86 does not define ARCH_HAS_DEV_MEM either, so > should see the same problem? > > On Fri, 2007-07-06 at 10:15 -0700, Xen patchbot-linux-2.6.18-xen wrote: >> # HG changeset patch >> # User kfraser@localhost.localdomain >> # Date 1183728779 -3600 >> # Node ID 4a08141e62ca189577041d8854bb478e5fbe502f >> # Parent 9e66b8728bd3a5d857500117c421051ae7dd0d61 >> Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations. >> Signed-off-by: Jun Kamada <kama@jp.fujitsu.com> >> --- >> drivers/xen/Kconfig | 4 ++++ >> drivers/xen/char/Makefile | 3 +-- >> drivers/xen/char/mem.c | 4 ++++ >> 3 files changed, 9 insertions(+), 2 deletions(-) >> >> diff -r 9e66b8728bd3 -r 4a08141e62ca drivers/xen/Kconfig >> --- a/drivers/xen/Kconfig Fri Jul 06 14:01:27 2007 +0100 >> +++ b/drivers/xen/Kconfig Fri Jul 06 14:32:59 2007 +0100 >> @@ -281,4 +281,8 @@ config XEN_XENCOMM >> config XEN_XENCOMM >> bool >> >> +config XEN_DEVMEM >> + bool >> + default y >> + >> endif >> diff -r 9e66b8728bd3 -r 4a08141e62ca drivers/xen/char/Makefile >> --- a/drivers/xen/char/Makefile Fri Jul 06 14:01:27 2007 +0100 >> +++ b/drivers/xen/char/Makefile Fri Jul 06 14:32:59 2007 +0100 >> @@ -1,2 +1,1 @@ >> - >> -obj-y := mem.o >> +obj-$(CONFIG_XEN_DEVMEM) := mem.o >> diff -r 9e66b8728bd3 -r 4a08141e62ca drivers/xen/char/mem.c >> --- a/drivers/xen/char/mem.c Fri Jul 06 14:01:27 2007 +0100 >> +++ b/drivers/xen/char/mem.c Fri Jul 06 14:32:59 2007 +0100 >> @@ -33,6 +33,7 @@ static inline int valid_phys_addr_range( >> } >> #endif >> >> +#ifdef ARCH_HAS_DEV_MEM >> /* >> * This funcion reads the *physical* memory. The f_pos points directly to >> the >> * memory location. >> @@ -135,6 +136,7 @@ static ssize_t write_mem(struct file * f >> *ppos += written; >> return written; >> } >> +#endif >> >> #ifndef ARCH_HAS_DEV_MEM_MMAP_MEM >> static inline int uncached_access(struct file *file) >> @@ -194,6 +196,7 @@ static int open_mem(struct inode * inode >> return capable(CAP_SYS_RAWIO) ? 0 : -EPERM; >> } >> >> +#ifdef ARCH_HAS_DEV_MEM >> const struct file_operations mem_fops = { >> .llseek = memory_lseek, >> .read = read_mem, >> @@ -201,3 +204,4 @@ const struct file_operations mem_fops >> .mmap = xen_mmap_mem, >> .open = open_mem, >> }; >> +#endif >> >> _______________________________________________ >> Xen-changelog mailing list >> Xen-changelog@lists.xensource.com >> http://lists.xensource.com/xen-changelog_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2007-Jul-09 19:20 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On Sat, 2007-07-07 at 10:01 +0100, Keir Fraser wrote:> > By the way, I wonder how PPC manages to build both drivers/char/mem.c and > drivers/xen/char/mem.c without ARCH_HAS_DEV_MEM? The model is supposed to be > that mem_fops defined by the Xen file is picked up by the generic file. If > !ARCH_HAS_DEV_MEM then that doesn''t happen -- so who picks up the Xen > mem_fops?Hmmm, yeah. Looks like we haven''t tested that... :) -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jul-09 19:26 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On 9/7/07 20:20, "Hollis Blanchard" <hollisb@us.ibm.com> wrote:>> By the way, I wonder how PPC manages to build both drivers/char/mem.c and >> drivers/xen/char/mem.c without ARCH_HAS_DEV_MEM? The model is supposed to be >> that mem_fops defined by the Xen file is picked up by the generic file. If >> !ARCH_HAS_DEV_MEM then that doesn''t happen -- so who picks up the Xen >> mem_fops? > > Hmmm, yeah. Looks like we haven''t tested that... :)If you don''t need to build both then there is potentially no problem with the Xen file hijacking the xlate_dev_mem() functions. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2007-Jul-09 19:41 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On Mon, 2007-07-09 at 20:26 +0100, Keir Fraser wrote:> On 9/7/07 20:20, "Hollis Blanchard" <hollisb@us.ibm.com> wrote: > > >> By the way, I wonder how PPC manages to build both drivers/char/mem.c and > >> drivers/xen/char/mem.c without ARCH_HAS_DEV_MEM? The model is supposed to be > >> that mem_fops defined by the Xen file is picked up by the generic file. If > >> !ARCH_HAS_DEV_MEM then that doesn''t happen -- so who picks up the Xen > >> mem_fops? > > > > Hmmm, yeah. Looks like we haven''t tested that... :) > > If you don''t need to build both then there is potentially no problem with > the Xen file hijacking the xlate_dev_mem() functions.PowerPC Linux builds support for multiple platforms, including multiple hypervisors, into a single binary (paravirt_ops was inspired by ppc_md). For the most part our Xen patches continue this model, but because we don''t really test our Xen kernel on non-Xen platforms, there are a couple bugs that would need fixing. So compile-time ifdefs for /dev/mem won''t work long-term, but I''m not really worried about it. -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jimi Xenidis
2007-Jul-09 20:23 UTC
Re: [XenPPC] Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On Jul 9, 2007, at 3:41 PM, Hollis Blanchard wrote:> On Mon, 2007-07-09 at 20:26 +0100, Keir Fraser wrote: >> On 9/7/07 20:20, "Hollis Blanchard" <hollisb@us.ibm.com> wrote: >> >>>> By the way, I wonder how PPC manages to build both drivers/char/ >>>> mem.c and >>>> drivers/xen/char/mem.c without ARCH_HAS_DEV_MEM? The model is >>>> supposed to be >>>> that mem_fops defined by the Xen file is picked up by the >>>> generic file. If >>>> !ARCH_HAS_DEV_MEM then that doesn''t happen -- so who picks up >>>> the Xen >>>> mem_fops? >>> >>> Hmmm, yeah. Looks like we haven''t tested that... :)yeah, we have :) xlate_dev_mem_ptr() works just fine for us, so drivers/char/mem.c just works and the xen version of the ops is never considered. Actually we should be forcing CONFIG_XEN_DEVMEM, but the Makefile ignores it. -JX _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jimi Xenidis
2007-Jul-09 21:20 UTC
Re: [XenPPC] Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On Jul 9, 2007, at 4:23 PM, Jimi Xenidis wrote:> > On Jul 9, 2007, at 3:41 PM, Hollis Blanchard wrote: > >> On Mon, 2007-07-09 at 20:26 +0100, Keir Fraser wrote: >>> On 9/7/07 20:20, "Hollis Blanchard" <hollisb@us.ibm.com> wrote: >>> >>>>> By the way, I wonder how PPC manages to build both drivers/char/ >>>>> mem.c and >>>>> drivers/xen/char/mem.c without ARCH_HAS_DEV_MEM? The model is >>>>> supposed to be >>>>> that mem_fops defined by the Xen file is picked up by the >>>>> generic file. If >>>>> !ARCH_HAS_DEV_MEM then that doesn''t happen -- so who picks up >>>>> the Xen >>>>> mem_fops? >>>> >>>> Hmmm, yeah. Looks like we haven''t tested that... :) > > yeah, we have :) > xlate_dev_mem_ptr() works just fine for us, so drivers/char/mem.c > just works and the xen version of the ops is never considered. > > Actually we should be forcing CONFIG_XEN_DEVMEM, but the Makefile > ignores it.s/CONFIG_XEN_DEVMEM/CONFIG_XEN_DEVMEM=n/> -JX > > _______________________________________________ > Xen-ppc-devel mailing list > Xen-ppc-devel@lists.xensource.com > http://lists.xensource.com/xen-ppc-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jun Kamada
2007-Jul-11 05:44 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
Keir-san and Hollis-san, On Sat, 07 Jul 2007 10:01:31 +0100 Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:> We define it in asm/mach-xen/asm/io.h. > > However, the patch *is* of questionable value. It guards the only non-static > definition in the file with #ifdef ARCH_HAS_DEV_MEM. Which begs the question > why you would build the file at all if !ARCH_HAS_DEV_MEM. I''ll revert it.I''m sorry that I had posted the incomplete patch. I will attach a new one modified. The patch resolves ploblems mentioned above? Best regards, ----- Jun Kamada Linux Technology Development Div. Server Systems Unit Fujitsu Ltd. kama@jp.fujitsu.com _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jul-11 10:00 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On 11/7/07 06:44, "Jun Kamada" <kama@jp.fujitsu.com> wrote:>> We define it in asm/mach-xen/asm/io.h. >> >> However, the patch *is* of questionable value. It guards the only non-static >> definition in the file with #ifdef ARCH_HAS_DEV_MEM. Which begs the question >> why you would build the file at all if !ARCH_HAS_DEV_MEM. I''ll revert it. > > I''m sorry that I had posted the incomplete patch. I will attach a new > one modified. The patch resolves ploblems mentioned above?The case of building drivers/xen/char/mem.c, yet not defining ARCH_HAS_DEV_MEM, does not seem useful. Who will pick up and use the mem_fops defined by drivers/xen/char/mem.c? At the very least this seems abusive of ARCH_HAS_DEV_MEM, and you might be better off defining a different macro name? But I think you need to explain to us what it is you''re actually trying to achieve. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jun Kamada
2007-Jul-12 11:05 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdef ARCH_HAS_DEV_MEM" to archtecture specific file_operations.
Keir-san, On Wed, 11 Jul 2007 11:00:37 +0100 Keir Fraser <keir@xensource.com> wrote:> The case of building drivers/xen/char/mem.c, yet not defining > ARCH_HAS_DEV_MEM, does not seem useful. Who will pick up and use the > mem_fops defined by drivers/xen/char/mem.c? > > At the very least this seems abusive of ARCH_HAS_DEV_MEM, and you might be > better off defining a different macro name? But I think you need to explain > to us what it is you''re actually trying to achieve.I would like to deal with the drivers/xen/char/mem.c as follows. How do you think about it? It will cause any problem? - I will post a patch, which removes definition of ARCH_HAS_DEV_MEM, to xen-ia64-devel later. - If needed, I will post a revert patch of "xen-ia64-devel.hg c/s 12544:395aa5609e6d". (Creating the revert patch may be difficult...)> better off defining a different macro name? But I think you need to explain > to us what it is you''re actually trying to achieve.We would like to map machine address to guest physical address at PCI initialization phase but at mmap system call in order to support Xwindow. The patch I posted is one of the patch which disables mapping process at mmap system call. Best regards, ----- Jun Kamada Linux Technology Development Div. Server Systems Unit Fujitsu Ltd. kama@jp.fujitsu.com _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Akio Takebe
2007-Jul-13 02:42 UTC
[Xen-ia64-devel] Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdefARCH_HAS_DEV_MEM" to archtecture specific file_operations.
Hi, Kama, Keir and Isaku>On Wed, 11 Jul 2007 11:00:37 +0100 >Keir Fraser <keir@xensource.com> wrote: >> The case of building drivers/xen/char/mem.c, yet not defining >> ARCH_HAS_DEV_MEM, does not seem useful. Who will pick up and use the >> mem_fops defined by drivers/xen/char/mem.c? >> >> At the very least this seems abusive of ARCH_HAS_DEV_MEM, and you might be >> better off defining a different macro name? But I think you need to explain >> to us what it is you''re actually trying to achieve. > >I would like to deal with the drivers/xen/char/mem.c as follows. How do >you think about it? It will cause any problem? > >- I will post a patch, which removes definition of ARCH_HAS_DEV_MEM, to > xen-ia64-devel later. >- If needed, I will post a revert patch of > "xen-ia64-devel.hg c/s 12544:395aa5609e6d". (Creating the revert patch > may be difficult...)Xen-ia64 already don''t need to modify drivers/xen/char/mem.c. But as you mentioned, current drivers/xen/char/mem.c has some parts for xen-ia64. So we may need to cleanup drivers/xen/char/mem.c. I made a attached revert patch of xen-unstablecs12513[1] (same as xen-ia64 cs12544). Keir, Isaku, how about it? If the patch is applied, we cannot compile linux-xen-ia64. But I have a patch of reverting cs12873[2], if the patch is also applied, we can compile linux-xen-ia64 again. I think Kama will post this patch to xen-ia64-devel soon. Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com> Signed-off-by: Jun Kamada <kama@jp.fujitsu.com> [1] cs12513 http://xenbits.xensource.com/xen-unstable.hg?rev/395aa5609e6d [2] cs12873 http://xenbits.xensource.com/xen-unstable.hg?rev/e5e6893ec699 Best Regards, Akio Takebe _______________________________________________ Xen-ia64-devel mailing list Xen-ia64-devel@lists.xensource.com http://lists.xensource.com/xen-ia64-devel
Keir Fraser
2007-Jul-13 12:54 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdefARCH_HAS_DEV_MEM" to archtecture specific file_operations.
I am happy to apply these if this is the right direction for ia64. Will someone else (e.g., Alex, Isaku) ack them if so, please? -- Keir On 13/7/07 03:42, "Akio Takebe" <takebe_akio@jp.fujitsu.com> wrote:> Xen-ia64 already don''t need to modify drivers/xen/char/mem.c. > But as you mentioned, current drivers/xen/char/mem.c has some parts > for xen-ia64. So we may need to cleanup drivers/xen/char/mem.c. > > I made a attached revert patch of xen-unstablecs12513[1] > (same as xen-ia64 cs12544). Keir, Isaku, how about it?_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Isaku Yamahata
2007-Jul-13 14:54 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdefARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On Fri, Jul 13, 2007 at 01:54:10PM +0100, Keir Fraser wrote:> > I am happy to apply these if this is the right direction for ia64. Will > someone else (e.g., Alex, Isaku) ack them if so, please?Fine for me. acked-by: Isaku Yamahata <yamahata@valinux.co.jp> -- yamahata _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Jul-13 15:09 UTC
Re: [Xen-devel] Re: [Xen-changelog] [linux-2.6.18-xen] Add "#ifdefARCH_HAS_DEV_MEM" to archtecture specific file_operations.
On 13/7/07 15:54, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:> On Fri, Jul 13, 2007 at 01:54:10PM +0100, Keir Fraser wrote: >> >> I am happy to apply these if this is the right direction for ia64. Will >> someone else (e.g., Alex, Isaku) ack them if so, please? > > Fine for me. > acked-by: Isaku Yamahata <yamahata@valinux.co.jp>Okay, revert of 12513 is now applied. I assume the other patch will be applied to the ia64 linux tree. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel