WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3

To: Jan Beulich <jbeulich@xxxxxxxxxx>
Subject: RE: [Xen-devel] Re: [PATCH][VTD] pci mmcfg patch for x86-64 - version 3
From: "Kay, Allen M" <allen.m.kay@xxxxxxxxx>
Date: Thu, 11 Dec 2008 18:47:20 -0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "Han, Weidong" <weidong.han@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Espen Skoglund <espen.skoglund@xxxxxxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Delivery-date: Thu, 11 Dec 2008 18:47:52 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <493F9376.76E4.0078.0@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <57C9024A16AD2D4C97DC78E552063EA35C7EF156@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <493F9376.76E4.0078.0@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AclaperCSVSWT0kgTxSc2kFjKGXzOQBWYFBA
Thread-topic: [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