As part of my endless quest to enable GART/IOMMU, I realized I need to make a slight change to a static function inside of agp-amd64.c. Currently Xen doesn''t have -xen variants of the AGP code. Is there a better way to handle this than sucking in the entire AGP tree into xen-sparse? As far what I need to change: pci-gart calls agp_amd64_init() to determine if the aperture is provided by the BIOS, or if one needs to be allocated. agp_amd64_init() calls agp_amd64_probe() which calls another function and so forth, and eventually aperture_valid() calls PageReserved(pfn_to_page(aperture >> PAGE_SHIFT)). The page isn''t actually reserved, but dom0 thinks it is, and the operation fails. I would like to do something more intelligent. -Mark Langsdorf Operating Systems Research Center AMD, Inc. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> "Langsdorf, Mark" <mark.langsdorf@amd.com> 26.03.07 22:03 >>> >As part of my endless quest to enable GART/IOMMU, I >realized I need to make a slight change to a static >function inside of agp-amd64.c. Currently Xen doesn''t >have -xen variants of the AGP code. Is there a >better way to handle this than sucking in the entire >AGP tree into xen-sparse?If the change is as small as you describe, I''d suggest doing it in the file itself by means of adding a patch in patches/linux-2.6.18/, with the change properly protected by #ifdef CONFIG_XEN or alike.>As far what I need to change: > pci-gart calls agp_amd64_init() to determine if >the aperture is provided by the BIOS, or if one >needs to be allocated. agp_amd64_init() calls >agp_amd64_probe() which calls another function >and so forth, and eventually aperture_valid() >calls >PageReserved(pfn_to_page(aperture >> PAGE_SHIFT)). >The page isn''t actually reserved, but dom0 thinks >it is, and the operation fails. I would like to >do something more intelligent.>From that description I''m getting afraid that this code is currentlybroken anyway, i.e. the change you intend to make is needed immediately and regardless of your iommu work. May I ask what your intended replacement is? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> "Langsdorf, Mark" <mark.langsdorf@amd.com> 26.03.07 22:03 >>> >As part of my endless quest to enable GART/IOMMU, I >realized I need to make a slight change to a static >function inside of agp-amd64.c. Currently Xen doesn''t >have -xen variants of the AGP code. Is there a >better way to handle this than sucking in the entire >AGP tree into xen-sparse? > >As far what I need to change: > pci-gart calls agp_amd64_init() to determine if >the aperture is provided by the BIOS, or if one >needs to be allocated. agp_amd64_init() calls >agp_amd64_probe() which calls another function >and so forth, and eventually aperture_valid() >calls >PageReserved(pfn_to_page(aperture >> PAGE_SHIFT)). >The page isn''t actually reserved, but dom0 thinks >it is, and the operation fails. I would like to >do something more intelligent.On a second look I believe the implementation is broken even on native, as long as !CONFIG_FLATMEM, since there''s an assumption that an invalid PFN cannot be followed by a valid one. For that reason, I think the code needs to be changed to call e820_any_mapped() (just like aperture.c does). I have a tentative patch to do that, but don''t have a working box with an 8151. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> >>> "Langsdorf, Mark" <mark.langsdorf@amd.com> 26.03.07 22:03 >>> > >As part of my endless quest to enable GART/IOMMU, I > >realized I need to make a slight change to a static > >function inside of agp-amd64.c. Currently Xen doesn''t > >have -xen variants of the AGP code. Is there a > >better way to handle this than sucking in the entire > >AGP tree into xen-sparse? > > > >As far what I need to change: > > pci-gart calls agp_amd64_init() to determine if > >the aperture is provided by the BIOS, or if one > >needs to be allocated. agp_amd64_init() calls > >agp_amd64_probe() which calls another function > >and so forth, and eventually aperture_valid() > >calls > >PageReserved(pfn_to_page(aperture >> PAGE_SHIFT)). > >The page isn''t actually reserved, but dom0 thinks > >it is, and the operation fails. I would like to > >do something more intelligent. > > On a second look I believe the implementation is broken even > on native, as long as !CONFIG_FLATMEM, since there''s an > assumption that an invalid PFN cannot be followed by a valid > one. For that reason, I think the code needs to be changed to > call e820_any_mapped() (just like aperture.c does). I have a > tentative patch to do that, but don''t have a working box with > an 8151.I do. You can send it to me for testing. I was playing with that box yesterday, and I discovered that Xen allocates guest virtual memory over the AGP aperture if dom0 memory is greater than 4G, even though the e820 map says it shouldn''t. I didn''t have a lot of spare cycles yesterday to deal with the implications of that, and maybe it can be safely ignored. Any thoughts? -Mark Langsdorf AMD, Inc. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 28/3/07 16:21, "Langsdorf, Mark" <mark.langsdorf@amd.com> wrote:> I was playing with that box yesterday, and I discovered > that Xen allocates guest virtual memory over the AGP > aperture if dom0 memory is greater than 4G, even though > the e820 map says it shouldn''t. I didn''t have a lot of > spare cycles yesterday to deal with the implications of > that, and maybe it can be safely ignored. Any thoughts?What exactly do you mean? That we allocate pages from the physical address range that includes the AGP aperture, even though this range is not marked as RAM in the e820 map? That would be a very bad bug. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 28.03.07 17:43 >>> >On 28/3/07 16:21, "Langsdorf, Mark" <mark.langsdorf@amd.com> wrote: > >> I was playing with that box yesterday, and I discovered >> that Xen allocates guest virtual memory over the AGP >> aperture if dom0 memory is greater than 4G, even though >> the e820 map says it shouldn''t. I didn''t have a lot of >> spare cycles yesterday to deal with the implications of >> that, and maybe it can be safely ignored. Any thoughts? > >What exactly do you mean? That we allocate pages from the physical address >range that includes the AGP aperture, even though this range is not marked >as RAM in the e820 map? That would be a very bad bug.I think he''s seeing PFNs used that match the MFNs of the aperture, which is exactly the bug I was referring to in my first reply - by switching the whole thing to use e820_any_mapped(), this problem would be gone. Otherwise, #idef CONFIG_XEN code will be needed in there. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>> On a second look I believe the implementation is broken even >> on native, as long as !CONFIG_FLATMEM, since there''s an >> assumption that an invalid PFN cannot be followed by a valid >> one. For that reason, I think the code needs to be changed to >> call e820_any_mapped() (just like aperture.c does). I have a >> tentative patch to do that, but don''t have a working box with >> an 8151. > >I do. You can send it to me for testing.Attached - depending on what tree you want to apply it on you may have to tweak it a little. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
> >> On a second look I believe the implementation is broken even > >> on native, as long as !CONFIG_FLATMEM, since there''s an > >> assumption that an invalid PFN cannot be followed by a valid > >> one. For that reason, I think the code needs to be changed to > >> call e820_any_mapped() (just like aperture.c does). I have a > >> tentative patch to do that, but don''t have a working box with > >> an 8151. > > > >I do. You can send it to me for testing. > > Attached - depending on what tree you want to apply it on you > may have to tweak it a little.I applied it to xen-unstable with some tweaking (my version doesn''t seem to have an i386 e820-xen.c ??) and to 2.6.20. System booted correctly and ran fine. Acked-by: Mark Langsdorf <mark.langsdorf@amd.com> -Mark Langsdorf Operating Systems Research Center AMD, Inc. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> "Langsdorf, Mark" <mark.langsdorf@amd.com> 29.03.07 00:37 >>> >> >> On a second look I believe the implementation is broken even >> >> on native, as long as !CONFIG_FLATMEM, since there''s an >> >> assumption that an invalid PFN cannot be followed by a valid >> >> one. For that reason, I think the code needs to be changed to >> >> call e820_any_mapped() (just like aperture.c does). I have a >> >> tentative patch to do that, but don''t have a working box with >> >> an 8151. >> > >> >I do. You can send it to me for testing. >> >> Attached - depending on what tree you want to apply it on you >> may have to tweak it a little. > >I applied it to xen-unstable with some tweaking (my version >doesn''t seem to have an i386 e820-xen.c ??) and to 2.6.20.Yes, i386''s e820.c is a 2.6.20 addition, which I followed in our Xen port. But I assume the patch is not likely to be applied to -unstable anyway, because of it touching a few files not in the sparse tree (unless Keir feels otherwise, or if your iommu stuff will have a strict dependency on this). The primary intention is to post the non-Xen parts to kernel.org and pick up when it got merged.>System booted correctly and ran fine. > >Acked-by: Mark Langsdorf <mark.langsdorf@amd.com>Thanks! Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel