|
|
|
|
|
|
|
|
|
|
xen-devel
RE: [Xen-devel] Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3
>
>>--- a/xen/arch/x86/Makefile Tue Dec 09 16:28:02 2008 +0000
>>+++ b/xen/arch/x86/Makefile Tue Dec 09 06:23:29 2008 -0800
>>@@ -54,6 +54,9 @@ obj-y += crash.o
>> obj-y += crash.o
>> obj-y += tboot.o
>> obj-y += hpet.o
>>+obj-y += mmconfig-shared.o
>>+obj-$(CONFIG_X86_64) += mmconfig_64.o
>>+obj-$(CONFIG_X86_32) += mmconfig_32.o
>
>These should now really go into xen/arch/x86/x86_{32,64}/ rather than
>adding _{32,64} suffixes.
>
I was trying to maintain the same names as Linux for ease of reference. You
are right, it would be more appropriate to follow Xen directory structure here.
I also tried to maintain file format of mmconfig_x?? to conform to Linux as
they are pretty much straight copies of Linux files. Sounds like these file
should also be converted to Xen indentation here.
>>+int pci_mmcfg_read(unsigned int seg, unsigned int bus,
>>+ unsigned int devfn, int reg, int len,
>u32 *value)
>>+{
>>+ *value = -1;
>>+ return 0;
>>+}
>
>Shouldn't you return -EINVAL (or -ENOSYS, but an error in any
>case) here?
>
Yes it should return -EINVAL as in the original Linux code. The change to 0
was unintentional.
>And one thing I didn't notice before:
>
>>+/* used by mmcfg */
>>+static inline unsigned char mmio_config_readb(void __iomem *pos)
>>+{
>>...
>
>This is preceded by a rather important comment regarding AMD Fam10
>CPUs in Linux. Without that comment, no-one will easily understand why
>you must use eax/ax/al here. I'm also surprised you didn't copy over
>pci_mmcfg_amd_fam10h() from Linux...
>
OK, I will add the comments back in. Where is pci_mmcfg_amd_family10h()
defined? I'm having trouble finding it.
>Oh, and one more thing I remembered just now: Linux verifies the mmcfg
>values it gets from ACPI against the E820 map - shouldn't Xen
>do this too?
My original intent was to keep mmconfig-shared.c and mmconfig_64.c as closely
to Linux as possible. However, I soon found out including everything in
mmconfig-shared.c involved pulling in a lot of code from Linux.
As my main goal of this round is enabling ATS, I tried to limited the scope of
mmconfig work to be somewhat manageable for now and then add more stuff to it
as needed. As I'm planning to work on multi-segment PCI support in Q1, this
stuff will get revisited again.
Other than E820 checking, do you see anything else in mmconfig-shared.c need to
be included for this round?
Allen
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
|
|
|
|