[Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

Map 3G (i440fx) or 2G (q35) of memory below 4G, so the RAM pieces
are nicely aligned to gigabyte borders.

Keep old memory layout for (a) old machine types and (b) in case all
memory fits below 4G and thus we don't have to split RAM into pieces
in the first place.  The later makes sure this change doesn't take
away memory from 32bit guests.

So, with i440fx and up to 3.5 GB of memory, all of it will be mapped
below 4G.  With more than 3.5 GB of memory 3 GB will be mapped below
4G and the remaining amount will be mapped above 4G.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
v2: rebased, resolved comflicts, resent as non-rfc
---
 hw/i386/pc_piix.c | 7 +++++--
 hw/i386/pc_q35.c  | 7 +++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4e0dae7..acb9445 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,6 +61,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 static bool has_pci_info;
 static bool has_acpi_build = true;
 static bool smbios_type1_defaults = true;
+static bool gigabyte_align = true;
 
 /* PC hardware initialisation */
 static void pc_init1(QEMUMachineInitArgs *args,
@@ -107,8 +108,9 @@ static void pc_init1(QEMUMachineInitArgs *args,
     }
 
     if (args->ram_size >= 0xe0000000) {
-        above_4g_mem_size = args->ram_size - 0xe0000000;
-        below_4g_mem_size = 0xe0000000;
+        ram_addr_t lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000;
+        above_4g_mem_size = args->ram_size - lowmem;
+        below_4g_mem_size = lowmem;
     } else {
         above_4g_mem_size = 0;
         below_4g_mem_size = args->ram_size;
@@ -245,6 +247,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
     smbios_type1_defaults = false;
+    gigabyte_align = false;
 }
 
 static void pc_compat_1_6(QEMUMachineInitArgs *args)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 07f38ff..7104645 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -51,6 +51,7 @@
 static bool has_pci_info;
 static bool has_acpi_build = true;
 static bool smbios_type1_defaults = true;
+static bool gigabyte_align = true;
 
 /* PC hardware initialisation */
 static void pc_q35_init(QEMUMachineInitArgs *args)
@@ -93,8 +94,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     kvmclock_create();
 
     if (args->ram_size >= 0xb0000000) {
-        above_4g_mem_size = args->ram_size - 0xb0000000;
-        below_4g_mem_size = 0xb0000000;
+        ram_addr_t lowmem = gigabyte_align ? 0x80000000 : 0xb0000000;
+        above_4g_mem_size = args->ram_size - lowmem;
+        below_4g_mem_size = lowmem;
     } else {
         above_4g_mem_size = 0;
         below_4g_mem_size = args->ram_size;
@@ -228,6 +230,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 static void pc_compat_1_7(QEMUMachineInitArgs *args)
 {
     smbios_type1_defaults = false;
+    gigabyte_align = false;
 }
 
 static void pc_compat_1_6(QEMUMachineInitArgs *args)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Mon, Dec 16, 2013 at 10:11:28AM +0100, Gerd Hoffmann wrote:
> Map 3G (i440fx) or 2G (q35) of memory below 4G, so the RAM pieces
> are nicely aligned to gigabyte borders.
> 
> Keep old memory layout for (a) old machine types and (b) in case all
> memory fits below 4G and thus we don't have to split RAM into pieces
> in the first place.  The later makes sure this change doesn't take
> away memory from 32bit guests.
> 
> So, with i440fx and up to 3.5 GB of memory, all of it will be mapped
> below 4G.  With more than 3.5 GB of memory 3 GB will be mapped below
> 4G and the remaining amount will be mapped above 4G.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>


OK the piix part looks ok to me.  For now I split this
int two patches: for q35 and piix and parked this on my PCI tree.
pushed, so pls check it out.
I also added some comments - see patches I've sent on list.

However, I'm not sure why do we reserve so much memory for q35.

I re-checked the pci express spec, it explicitly says
(Table 7-1: Enhanced Configuration Address Mapping)
that address bits used for ECAM (aka MMCFG) are 20 + n - 1 to 0,
wheren n is bits in the bus number field, so up to 8.
Doing the math we need up to 28 bits that is 256 Megabytes
of memory.

So what's the issue with using up to 3G for RAM?
This makes me think the only issue is that it seems to conflict
with MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT, which we should just get rid of -
it's never actually used.

Does the analysis above make sense to you?

-- 
MST

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

On Mo, 2013-12-16 at 13:54 +0200, Michael S. Tsirkin wrote:
> On Mon, Dec 16, 2013 at 10:11:28AM +0100, Gerd Hoffmann wrote:
> > Map 3G (i440fx) or 2G (q35) of memory below 4G, so the RAM pieces
> > are nicely aligned to gigabyte borders.
> > 
> > Keep old memory layout for (a) old machine types and (b) in case all
> > memory fits below 4G and thus we don't have to split RAM into pieces
> > in the first place.  The later makes sure this change doesn't take
> > away memory from 32bit guests.
> > 
> > So, with i440fx and up to 3.5 GB of memory, all of it will be mapped
> > below 4G.  With more than 3.5 GB of memory 3 GB will be mapped below
> > 4G and the remaining amount will be mapped above 4G.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> 
> OK the piix part looks ok to me.  For now I split this
> int two patches: for q35 and piix and parked this on my PCI tree.
> pushed, so pls check it out.
> I also added some comments - see patches I've sent on list.
> 
> However, I'm not sure why do we reserve so much memory for q35.
> 
> I re-checked the pci express spec, it explicitly says
> (Table 7-1: Enhanced Configuration Address Mapping)
> that address bits used for ECAM (aka MMCFG) are 20 + n - 1 to 0,
> wheren n is bits in the bus number field, so up to 8.
> Doing the math we need up to 28 bits that is 256 Megabytes
> of memory.

Correct.

> So what's the issue with using up to 3G for RAM?
> This makes me think the only issue is that it seems to conflict
> with MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT, which we should just get rid of -
> it's never actually used.

Indeed, that has no use because the firmware initializes the xbar
anyway.

Problem is that the firmware places the xbar @ 0xb000000.
Hardcoded, assuming qemu will not map ram above 0xb0000000.

So, we must (a) fix firmware first and (b) get a ugly dependency
that older firmware will not run on latest qemu.

We also need to figure how we wanna fixup things.  So, current memory
layout looks like this:

   0x00000000 - 0xafffffff  --  RAM / unused
   0xb0000000 - 0xbfffffff  --  mmconfig xbar [256 pci busses]
   0xc0000000 - 0xfec00000  --  space for pci bars, almost 1g

Largest pci bar we can map below 4g is 512m, @ 0xc0000000.

If we wanna map 3G RAM we need to move the xbar somewhere else.  Big
question is where?

We can move it to 0xc0000000.  Then we can't map 512m pci bars any more.

We can move it to 0xe0000000.  Then we have to split the pci bar space,
mapping large bars below 0xe0000000 and small ones above 0xf0000000.
SeaBIOS pci init code isn't really up to it.  Could also become tricky
to declare it correctly in acpi / e820 due to the split.

We can move it to 0xf0000000 or 0xf8000000, then map all pci bars below
the xbar.  We have to make the xbar smaller then, reducing the number of
pci busses the mmconf area can handle.  My laptop looks like this.  Not
sure this a good move for qemu though, do we really want to limit the
number of busses we can handle, with everything moving to pci express?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Mon, Dec 16, 2013 at 02:46:17PM +0100, Gerd Hoffmann wrote:
> On Mo, 2013-12-16 at 13:54 +0200, Michael S. Tsirkin wrote:
> > On Mon, Dec 16, 2013 at 10:11:28AM +0100, Gerd Hoffmann wrote:
> > > Map 3G (i440fx) or 2G (q35) of memory below 4G, so the RAM pieces
> > > are nicely aligned to gigabyte borders.
> > > 
> > > Keep old memory layout for (a) old machine types and (b) in case all
> > > memory fits below 4G and thus we don't have to split RAM into pieces
> > > in the first place.  The later makes sure this change doesn't take
> > > away memory from 32bit guests.
> > > 
> > > So, with i440fx and up to 3.5 GB of memory, all of it will be mapped
> > > below 4G.  With more than 3.5 GB of memory 3 GB will be mapped below
> > > 4G and the remaining amount will be mapped above 4G.
> > > 
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > 
> > OK the piix part looks ok to me.  For now I split this
> > int two patches: for q35 and piix and parked this on my PCI tree.
> > pushed, so pls check it out.
> > I also added some comments - see patches I've sent on list.
> > 
> > However, I'm not sure why do we reserve so much memory for q35.
> > 
> > I re-checked the pci express spec, it explicitly says
> > (Table 7-1: Enhanced Configuration Address Mapping)
> > that address bits used for ECAM (aka MMCFG) are 20 + n - 1 to 0,
> > wheren n is bits in the bus number field, so up to 8.
> > Doing the math we need up to 28 bits that is 256 Megabytes
> > of memory.
> 
> Correct.
> 
> > So what's the issue with using up to 3G for RAM?
> > This makes me think the only issue is that it seems to conflict
> > with MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT, which we should just get rid of -
> > it's never actually used.
> 
> Indeed, that has no use because the firmware initializes the xbar
> anyway.
> 
> Problem is that the firmware places the xbar @ 0xb000000.
> Hardcoded, assuming qemu will not map ram above 0xb0000000.

Can't bios figure out the size of memory below 4G from fwcfg?
I refer to 7db16f2480db5e246d34d0c453cff4f58549df0e specifically.


> So, we must (a) fix firmware first and (b) get a ugly dependency
> that older firmware will not run on latest qemu.

That's only important for old machine types though, right?

> We also need to figure how we wanna fixup things.  So, current memory
> layout looks like this:
> 
>    0x00000000 - 0xafffffff  --  RAM / unused
>    0xb0000000 - 0xbfffffff  --  mmconfig xbar [256 pci busses]
>    0xc0000000 - 0xfec00000  --  space for pci bars, almost 1g
> 
> Largest pci bar we can map below 4g is 512m, @ 0xc0000000.
> 
> If we wanna map 3G RAM we need to move the xbar somewhere else.  Big
> question is where?
> 
> We can move it to 0xc0000000.  Then we can't map 512m pci bars any more.

I would go for this when we have 3G of RAM.
I think that ability to support a single 512m BAR is not all that important.

> 
> We can move it to 0xe0000000.  Then we have to split the pci bar space,
> mapping large bars below 0xe0000000 and small ones above 0xf0000000.
> SeaBIOS pci init code isn't really up to it.
> Could also become tricky
> to declare it correctly in acpi / e820 due to the split.

My laptop's ACPI has this space all fragmented up, seems to boot fine ...

> We can move it to 0xf0000000 or 0xf8000000, then map all pci bars below
> the xbar.  We have to make the xbar smaller then, reducing the number of
> pci busses the mmconf area can handle.  My laptop looks like this.  Not
> sure this a good move for qemu though, do we really want to limit the
> number of busses we can handle, with everything moving to pci express?
> 
> cheers,
>   Gerd

I don't think it's good. With bus per device we are limited to 256
devices as it is.

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

  Hi,

> > Problem is that the firmware places the xbar @ 0xb000000.
> > Hardcoded, assuming qemu will not map ram above 0xb0000000.
> 
> Can't bios figure out the size of memory below 4G from fwcfg?
> I refer to 7db16f2480db5e246d34d0c453cff4f58549df0e specifically.

It can, but it doesn't.

Additional issue for coreboot is that mmconfig base is a compile-time
constant, because it is setup _very_ early in the boot process.
Coreboot then does the whole pci initialization using mmconfig.

On the other hand coreboot has a much more sophisticated ressource
management than seabios, so moving the mmconf xbar up to to
0xe0000000-0xefffffff, then managing two regions (below 0xe0000000 and
above 0xf0000000) for pci bars probably isn't a big issue for coreboot.

> > So, we must (a) fix firmware first and (b) get a ugly dependency
> > that older firmware will not run on latest qemu.
> 
> That's only important for old machine types though, right?

Correct.  That makes it a bit less problematic, but it is still not very
nice.

> > We also need to figure how we wanna fixup things.  So, current memory
> > layout looks like this:
> > 
> >    0x00000000 - 0xafffffff  --  RAM / unused
> >    0xb0000000 - 0xbfffffff  --  mmconfig xbar [256 pci busses]
> >    0xc0000000 - 0xfec00000  --  space for pci bars, almost 1g
> > 
> > Largest pci bar we can map below 4g is 512m, @ 0xc0000000.
> > 
> > If we wanna map 3G RAM we need to move the xbar somewhere else.  Big
> > question is where?
> > 
> > We can move it to 0xc0000000.  Then we can't map 512m pci bars any more.
> 
> I would go for this when we have 3G of RAM.
> I think that ability to support a single 512m BAR is not all that important.

Use case: pci passthrough of graphics cards.

> > We can move it to 0xe0000000.  Then we have to split the pci bar space,
> > mapping large bars below 0xe0000000 and small ones above 0xf0000000.
> > SeaBIOS pci init code isn't really up to it.
> > Could also become tricky
> > to declare it correctly in acpi / e820 due to the split.
> 
> My laptop's ACPI has this space all fragmented up, seems to boot fine ...

We need to change the way we reserve the mmconfig space though.  

Currently it is marked reserved in the e820 table.  Having that overlap
with the _CRS region makes windows quite unhappy, we tried that
recently.

My laptop has the mmconfig space declared as LPC ressource:

            Device (LPC)
            {
                Name (_ADR, 0x001F0000)  // _ADR: Address
                Name (_S3D, 0x03)  // _S3D: S3 Device State
                Name (RID, 0x00)
                Device (SIO)
                {
                    Name (_HID, EisaId ("PNP0C02"))
                    Name (_UID, 0x00)  // _UID: Unique ID
                    Name (SCRS, ResourceTemplate ()
[ ... ]
                        Memory32Fixed (ReadWrite,
                            0xF8000000,         // Address Base
                            0x04000000,         // Address Length
                            )
[ ... ]
                    Method (_CRS, 0, NotSerialized)
[ ... return SCRS, with updates applied in some cases ... ]

When doing it this way we can simply make the PCI0._CRS cover the whole
end-of-ram -> ioapic-base range, simliar to piix, and we are pretty free
to place the mmconfig xbar anywhere in that area.

Doing the transition is non-trivial though because we (a) move the job
of reserving the mmconfig area from firmware to qemu and (b) the testing
needed for that.

Maybe we should set the gbyte alignment on q35 aside for a while and
tackle the mmconfig reservation handling first.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Tue, Dec 17, 2013 at 11:54:46AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > Problem is that the firmware places the xbar @ 0xb000000.
> > > Hardcoded, assuming qemu will not map ram above 0xb0000000.
> > 
> > Can't bios figure out the size of memory below 4G from fwcfg?
> > I refer to 7db16f2480db5e246d34d0c453cff4f58549df0e specifically.
> 
> It can, but it doesn't.
> 
> Additional issue for coreboot is that mmconfig base is a compile-time
> constant, because it is setup _very_ early in the boot process.
> Coreboot then does the whole pci initialization using mmconfig.
> 
> On the other hand coreboot has a much more sophisticated ressource
> management than seabios, so moving the mmconf xbar up to to
> 0xe0000000-0xefffffff, then managing two regions (below 0xe0000000 and
> above 0xf0000000) for pci bars probably isn't a big issue for coreboot.
> 
> > > So, we must (a) fix firmware first and (b) get a ugly dependency
> > > that older firmware will not run on latest qemu.
> > 
> > That's only important for old machine types though, right?
> 
> Correct.  That makes it a bit less problematic, but it is still not very
> nice.
> 
> > > We also need to figure how we wanna fixup things.  So, current memory
> > > layout looks like this:
> > > 
> > >    0x00000000 - 0xafffffff  --  RAM / unused
> > >    0xb0000000 - 0xbfffffff  --  mmconfig xbar [256 pci busses]
> > >    0xc0000000 - 0xfec00000  --  space for pci bars, almost 1g
> > > 
> > > Largest pci bar we can map below 4g is 512m, @ 0xc0000000.
> > > 
> > > If we wanna map 3G RAM we need to move the xbar somewhere else.  Big
> > > question is where?
> > > 
> > > We can move it to 0xc0000000.  Then we can't map 512m pci bars any more.
> > 
> > I would go for this when we have 3G of RAM.
> > I think that ability to support a single 512m BAR is not all that important.
> 
> Use case: pci passthrough of graphics cards.
> 
> > > We can move it to 0xe0000000.  Then we have to split the pci bar space,
> > > mapping large bars below 0xe0000000 and small ones above 0xf0000000.
> > > SeaBIOS pci init code isn't really up to it.
> > > Could also become tricky
> > > to declare it correctly in acpi / e820 due to the split.
> > 
> > My laptop's ACPI has this space all fragmented up, seems to boot fine ...
> 
> We need to change the way we reserve the mmconfig space though.  
> 
> Currently it is marked reserved in the e820 table.  Having that overlap
> with the _CRS region makes windows quite unhappy, we tried that
> recently.

Yes this also contradicts the spec, see below.

> My laptop has the mmconfig space declared as LPC ressource:
> 
>             Device (LPC)
>             {
>                 Name (_ADR, 0x001F0000)  // _ADR: Address
>                 Name (_S3D, 0x03)  // _S3D: S3 Device State
>                 Name (RID, 0x00)
>                 Device (SIO)
>                 {
>                     Name (_HID, EisaId ("PNP0C02"))
>                     Name (_UID, 0x00)  // _UID: Unique ID
>                     Name (SCRS, ResourceTemplate ()
> [ ... ]
>                         Memory32Fixed (ReadWrite,
>                             0xF8000000,         // Address Base
>                             0x04000000,         // Address Length
>                             )
> [ ... ]
>                     Method (_CRS, 0, NotSerialized)
> [ ... return SCRS, with updates applied in some cases ... ]
> 
> When doing it this way we can simply make the PCI0._CRS cover the whole
> end-of-ram -> ioapic-base range, simliar to piix, and we are pretty free
> to place the mmconfig xbar anywhere in that area.

The spec says:
	2.If the operating system does not natively comprehend reserving the
	MMCFG region, the
	MMCFG region must be reserved by firmware. The address range reported in
	the MCFG table
	or by _CBA method (see Section 4.1.3) must be reserved by declaring a
	motherboard resource.
	For most systems, the motherboard resource would appear at the root of
	the ACPI namespace
	(under \_SB) in a node with a _HID of EISAID (PNP0C02), and the
	resources in this case
	should not be claimed in the root PCI bus’s _CRS. The resources can
	optionally be returned in
	Int15 E820 or EFIGetMemoryMap as reserved memory but must always be
	reported through
	ACPI as a motherboard resource.

My reading of the above is that this can be an LPC resource but
claiming this as the root's _CRS isn't ok then.

> 
> Doing the transition is non-trivial though because we (a) move the job
> of reserving the mmconfig area from firmware to qemu and (b) the testing
> needed for that.
> 
> Maybe we should set the gbyte alignment on q35 aside for a while and
> tackle the mmconfig reservation handling first.
> 
> cheers,
>   Gerd

I merged your patch but split it: q35 is separate and piix
is separate. Would you like me to drop the q35 part then?

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

  Hi,

> > We need to change the way we reserve the mmconfig space though.  
> > 
> > Currently it is marked reserved in the e820 table.  Having that overlap
> > with the _CRS region makes windows quite unhappy, we tried that
> > recently.
> 
> Yes this also contradicts the spec, see below.
> 
> > My laptop has the mmconfig space declared as LPC ressource:
> > 
> >             Device (LPC)
> >             {
> >                 Name (_ADR, 0x001F0000)  // _ADR: Address
> >                 Name (_S3D, 0x03)  // _S3D: S3 Device State
> >                 Name (RID, 0x00)
> >                 Device (SIO)
> >                 {
> >                     Name (_HID, EisaId ("PNP0C02"))
> >                     Name (_UID, 0x00)  // _UID: Unique ID
> >                     Name (SCRS, ResourceTemplate ()
> > [ ... ]
> >                         Memory32Fixed (ReadWrite,
> >                             0xF8000000,         // Address Base
> >                             0x04000000,         // Address Length
> >                             )
> > [ ... ]
> >                     Method (_CRS, 0, NotSerialized)
> > [ ... return SCRS, with updates applied in some cases ... ]
> > 
> > When doing it this way we can simply make the PCI0._CRS cover the whole
> > end-of-ram -> ioapic-base range, simliar to piix, and we are pretty free
> > to place the mmconfig xbar anywhere in that area.
> 
> The spec says:

> 	(under \_SB) in a node with a _HID of EISAID (PNP0C02),

So this is what my laptop does.

>  and the
> 	resources in this case
> 	should not be claimed in the root PCI bus’s _CRS.

My laptop has them in the root bus _CRS though:

[    0.124634] PCI: MMCONFIG at [mem 0xf8000000-0xfbffffff] reserved in
ACPI motherboard resources

[    0.139391] pci_bus 0000:00: root bus resource [mem
0xdfa00000-0xfebfffff]

>  The resources can
> 	optionally be returned in
> 	Int15 E820 or EFIGetMemoryMap as reserved memory but must always be
> 	reported through
> 	ACPI as a motherboard resource.

So we can do both e820 and motherboard ressource.  Good, that hopefully
simplifies the transition.

> My reading of the above is that this can be an LPC resource but
> claiming this as the root's _CRS isn't ok then.

I read the specs the same way, but my laptop does something different.

Guess that needs quite some testing to figure which works best ...

> I merged your patch but split it: q35 is separate and piix
> is separate. Would you like me to drop the q35 part then?

If you are fine with q35 having only 2G lowmem keep it.  It's safe.

We can sort the mmconfig setup afterwards, then check if (and how) we'll
transition to 3G lowmem.  Maybe we simply don't after all, with the
world moving to 64bit it doesn't matter that much whenever memory is
mapped above or below 4g.  And for old 32bit guests there is always the
option to stick with piix which continues to offers up to 3.5G lowmem.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > We need to change the way we reserve the mmconfig space though.  
> > > 
> > > Currently it is marked reserved in the e820 table.  Having that overlap
> > > with the _CRS region makes windows quite unhappy, we tried that
> > > recently.
> > 
> > Yes this also contradicts the spec, see below.
> > 
> > > My laptop has the mmconfig space declared as LPC ressource:
> > > 
> > >             Device (LPC)
> > >             {
> > >                 Name (_ADR, 0x001F0000)  // _ADR: Address
> > >                 Name (_S3D, 0x03)  // _S3D: S3 Device State
> > >                 Name (RID, 0x00)
> > >                 Device (SIO)
> > >                 {
> > >                     Name (_HID, EisaId ("PNP0C02"))
> > >                     Name (_UID, 0x00)  // _UID: Unique ID
> > >                     Name (SCRS, ResourceTemplate ()
> > > [ ... ]
> > >                         Memory32Fixed (ReadWrite,
> > >                             0xF8000000,         // Address Base
> > >                             0x04000000,         // Address Length
> > >                             )
> > > [ ... ]
> > >                     Method (_CRS, 0, NotSerialized)
> > > [ ... return SCRS, with updates applied in some cases ... ]
> > > 
> > > When doing it this way we can simply make the PCI0._CRS cover the whole
> > > end-of-ram -> ioapic-base range, simliar to piix, and we are pretty free
> > > to place the mmconfig xbar anywhere in that area.
> > 
> > The spec says:
> 
> > 	(under \_SB) in a node with a _HID of EISAID (PNP0C02),
> 
> So this is what my laptop does.
> 
> >  and the
> > 	resources in this case
> > 	should not be claimed in the root PCI bus’s _CRS.
> 
> My laptop has them in the root bus _CRS though:
> 
> [    0.124634] PCI: MMCONFIG at [mem 0xf8000000-0xfbffffff] reserved in
> ACPI motherboard resources
> 
> [    0.139391] pci_bus 0000:00: root bus resource [mem
> 0xdfa00000-0xfebfffff]
> 
> >  The resources can
> > 	optionally be returned in
> > 	Int15 E820 or EFIGetMemoryMap as reserved memory but must always be
> > 	reported through
> > 	ACPI as a motherboard resource.
> 
> So we can do both e820 and motherboard ressource.  Good, that hopefully
> simplifies the transition.
> 
> > My reading of the above is that this can be an LPC resource but
> > claiming this as the root's _CRS isn't ok then.
> 
> I read the specs the same way, but my laptop does something different.
> 
> Guess that needs quite some testing to figure which works best ...
> 
> > I merged your patch but split it: q35 is separate and piix
> > is separate. Would you like me to drop the q35 part then?
> 
> If you are fine with q35 having only 2G lowmem keep it.  It's safe.
> 
> We can sort the mmconfig setup afterwards, then check if (and how) we'll
> transition to 3G lowmem.  Maybe we simply don't after all, with the
> world moving to 64bit it doesn't matter that much whenever memory is
> mapped above or below 4g.  And for old 32bit guests there is always the
> option to stick with piix which continues to offers up to 3.5G lowmem.
> 
> cheers,
>   Gerd
> 

I'll think it over, I will keep the patch around but won't merge
to Anthony meanwhile.

-- 
MST

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote:
> > I merged your patch but split it: q35 is separate and piix
> > is separate. Would you like me to drop the q35 part then?
> 
> If you are fine with q35 having only 2G lowmem keep it.  It's safe.
> 
> We can sort the mmconfig setup afterwards, then check if (and how) we'll
> transition to 3G lowmem.  Maybe we simply don't after all, with the
> world moving to 64bit it doesn't matter that much whenever memory is
> mapped above or below 4g.  And for old 32bit guests there is always the
> option to stick with piix which continues to offers up to 3.5G lowmem.
> 
> cheers,
>   Gerd
> 

Any update here?
I'm worried 2G lowmem is a bit too aggressive, PAE still exists.
If we want to support old bios, one way would be to add
a new register to enable 3g lowmem.
Or just ignore the issue with new machine types.

-- 
MST

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

On Mo, 2014-01-20 at 13:23 +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote:
> > > I merged your patch but split it: q35 is separate and piix
> > > is separate. Would you like me to drop the q35 part then?
> > 
> > If you are fine with q35 having only 2G lowmem keep it.  It's safe.
> > 
> > We can sort the mmconfig setup afterwards, then check if (and how) we'll
> > transition to 3G lowmem.  Maybe we simply don't after all, with the
> > world moving to 64bit it doesn't matter that much whenever memory is
> > mapped above or below 4g.  And for old 32bit guests there is always the
> > option to stick with piix which continues to offers up to 3.5G lowmem.
> > 
> > cheers,
> >   Gerd
> > 
> 
> Any update here?

No time to investigate yet, still playing catch-up after xmas holidays &
being sick.

> I'm worried 2G lowmem is a bit too aggressive, PAE still exists.

piix (with up to 3.5g lowmem) exists too ;)

> If we want to support old bios, one way would be to add
> a new register to enable 3g lowmem.

Do we really want do this (allow guest change RAM mapping)?

I think we should:
 (1) reserve mmconf xbar as motherboard ressource, so it can live within
     PCI0._CRS
 (2) make PCI0._CRS start at end of lowmem, like it does on piix

Then maybe:
 (3) update seabios to place xbar somewhere else
 (4) adjust memory layout.

(1)+(2) are useful anyway.  (3)+(4) would allow for 3g lowmem, and
obviously have some compatibility issues:  doing (4) requires a seabios
update.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Mon, Jan 20, 2014 at 01:58:45PM +0100, Gerd Hoffmann wrote:
> On Mo, 2014-01-20 at 13:23 +0200, Michael S. Tsirkin wrote:
> > On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote:
> > > > I merged your patch but split it: q35 is separate and piix
> > > > is separate. Would you like me to drop the q35 part then?
> > > 
> > > If you are fine with q35 having only 2G lowmem keep it.  It's safe.
> > > 
> > > We can sort the mmconfig setup afterwards, then check if (and how) we'll
> > > transition to 3G lowmem.  Maybe we simply don't after all, with the
> > > world moving to 64bit it doesn't matter that much whenever memory is
> > > mapped above or below 4g.  And for old 32bit guests there is always the
> > > option to stick with piix which continues to offers up to 3.5G lowmem.
> > > 
> > > cheers,
> > >   Gerd
> > > 
> > 
> > Any update here?
> 
> No time to investigate yet, still playing catch-up after xmas holidays &
> being sick.
> 
> > I'm worried 2G lowmem is a bit too aggressive, PAE still exists.
> 
> piix (with up to 3.5g lowmem) exists too ;)

Yes but I think it's preferable to keep features orthogonal.

> > If we want to support old bios, one way would be to add
> > a new register to enable 3g lowmem.
> 
> Do we really want do this (allow guest change RAM mapping)?
> 
> I think we should:
>  (1) reserve mmconf xbar as motherboard ressource, so it can live within
>      PCI0._CRS

I don't think this is possible, PCI FW spec seems to
outlaw this explicitly.

>  (2) make PCI0._CRS start at end of lowmem, like it does on piix

This we can do. But we'll have to make a hole for the xbar.

> Then maybe:
>  (3) update seabios to place xbar somewhere else
>  (4) adjust memory layout.
> 
> (1)+(2) are useful anyway.  (3)+(4) would allow for 3g lowmem, and
> obviously have some compatibility issues:  doing (4) requires a seabios
> update.
> 
> cheers,
>   Gerd

Right. So you'll look into 4 then? I'm keeping these
patches out of tree for now ...

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

On Mo, 2014-01-20 at 15:59 +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 20, 2014 at 01:58:45PM +0100, Gerd Hoffmann wrote:
> > On Mo, 2014-01-20 at 13:23 +0200, Michael S. Tsirkin wrote:
> > > On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote:
> > > > > I merged your patch but split it: q35 is separate and piix
> > > > > is separate. Would you like me to drop the q35 part then?
> > > > 
> > > > If you are fine with q35 having only 2G lowmem keep it.  It's safe.
> > > > 
> > > > We can sort the mmconfig setup afterwards, then check if (and how) we'll
> > > > transition to 3G lowmem.  Maybe we simply don't after all, with the
> > > > world moving to 64bit it doesn't matter that much whenever memory is
> > > > mapped above or below 4g.  And for old 32bit guests there is always the
> > > > option to stick with piix which continues to offers up to 3.5G lowmem.
> > > > 
> > > > cheers,
> > > >   Gerd
> > > > 
> > > 
> > > Any update here?
> > 
> > No time to investigate yet, still playing catch-up after xmas holidays &
> > being sick.
> > 
> > > I'm worried 2G lowmem is a bit too aggressive, PAE still exists.
> > 
> > piix (with up to 3.5g lowmem) exists too ;)
> 
> Yes but I think it's preferable to keep features orthogonal.
> 
> > > If we want to support old bios, one way would be to add
> > > a new register to enable 3g lowmem.
> > 
> > Do we really want do this (allow guest change RAM mapping)?
> > 
> > I think we should:
> >  (1) reserve mmconf xbar as motherboard ressource, so it can live within
> >      PCI0._CRS
> 
> I don't think this is possible, PCI FW spec seems to
> outlaw this explicitly.

Nevertheless my laptop does it this way, and it would simplify things
because we'll need a single mmio range below 4k then.

Have a pointer to the spec?

> Right. So you'll look into 4 then? I'm keeping these
> patches out of tree for now ...

Still plan to care when I find time, yes.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Mon, Jan 20, 2014 at 03:01:50PM +0100, Gerd Hoffmann wrote:
> On Mo, 2014-01-20 at 15:59 +0200, Michael S. Tsirkin wrote:
> > On Mon, Jan 20, 2014 at 01:58:45PM +0100, Gerd Hoffmann wrote:
> > > On Mo, 2014-01-20 at 13:23 +0200, Michael S. Tsirkin wrote:
> > > > On Tue, Dec 17, 2013 at 06:56:06PM +0100, Gerd Hoffmann wrote:
> > > > > > I merged your patch but split it: q35 is separate and piix
> > > > > > is separate. Would you like me to drop the q35 part then?
> > > > > 
> > > > > If you are fine with q35 having only 2G lowmem keep it.  It's safe.
> > > > > 
> > > > > We can sort the mmconfig setup afterwards, then check if (and how) we'll
> > > > > transition to 3G lowmem.  Maybe we simply don't after all, with the
> > > > > world moving to 64bit it doesn't matter that much whenever memory is
> > > > > mapped above or below 4g.  And for old 32bit guests there is always the
> > > > > option to stick with piix which continues to offers up to 3.5G lowmem.
> > > > > 
> > > > > cheers,
> > > > >   Gerd
> > > > > 
> > > > 
> > > > Any update here?
> > > 
> > > No time to investigate yet, still playing catch-up after xmas holidays &
> > > being sick.
> > > 
> > > > I'm worried 2G lowmem is a bit too aggressive, PAE still exists.
> > > 
> > > piix (with up to 3.5g lowmem) exists too ;)
> > 
> > Yes but I think it's preferable to keep features orthogonal.
> > 
> > > > If we want to support old bios, one way would be to add
> > > > a new register to enable 3g lowmem.
> > > 
> > > Do we really want do this (allow guest change RAM mapping)?
> > > 
> > > I think we should:
> > >  (1) reserve mmconf xbar as motherboard ressource, so it can live within
> > >      PCI0._CRS
> > 
> > I don't think this is possible, PCI FW spec seems to
> > outlaw this explicitly.
> 
> Nevertheless my laptop does it this way, and it would simplify things
> because we'll need a single mmio range below 4k then.
> 
> Have a pointer to the spec?

I refer to this:

4.1.2.
 MCFG Table Description


...

If the operating system does not natively comprehend reserving the MMCFG
region, the MMCFG region must be reserved by firmware. The address range
reported in the MCFG table or by _CBA method (see Section 4.1.3) must be
reserved by declaring a motherboard resource.  For most systems, the
motherboard resource would appear at the root of the ACPI namespace
(under \_SB) in a node with a _HID of EISAID (PNP0C02), and the
resources in this case should not be claimed in the root PCI bus’s _CRS.
The resources can optionally be returned in Int15 E820 or
EFIGetMemoryMap as reserved memory but must always be reported through
ACPI as a motherboard resource.


So if we care about legacy OS-es we must make MCFG a motherboard
resource and it must not be part of _CRS.


> > Right. So you'll look into 4 then? I'm keeping these
> > patches out of tree for now ...
> 
> Still plan to care when I find time, yes.
> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

> 4.1.2.
>  MCFG Table Description
> 
> 
> ...
> 
> If the operating system does not natively comprehend reserving the MMCFG
> region, the MMCFG region must be reserved by firmware. The address range
> reported in the MCFG table or by _CBA method (see Section 4.1.3) must be
> reserved by declaring a motherboard resource.

We don't do this today.

> For most systems, the
> motherboard resource would appear at the root of the ACPI namespace
> (under \_SB) in a node with a _HID of EISAID (PNP0C02), and the
> resources in this case should not be claimed in the root PCI bus’s _CRS.

Which I read as _in case it is at the root of the apci namespace_ it
should not be claimed in PCI0._CRS.  Which makes sense.

My laptop has it reserved in a \_SB\PCI0\LPC\SIO device instead:

            Device (LPC)
            {
                Name (_ADR, 0x001F0000)  // _ADR: Address
                Name (_S3D, 0x03)  // _S3D: S3 Device State
                Name (RID, 0x00)
                Device (SIO)
                {
                    Name (_HID, EisaId ("PNP0C02"))
                    Name (_UID, 0x00)  // _UID: Unique ID
                    Name (SCRS, ResourceTemplate ()
                    {
                        [ ... ]
                        Memory32Fixed (ReadWrite,
                            0xF8000000,         // Address Base
                            0x04000000,         // Address Length
                            )
                        [ ... ]

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Michael S. Tsirkin]

On Mon, Jan 20, 2014 at 04:36:57PM +0100, Gerd Hoffmann wrote:
> > 4.1.2.
> >  MCFG Table Description
> > 
> > 
> > ...
> > 
> > If the operating system does not natively comprehend reserving the MMCFG
> > region, the MMCFG region must be reserved by firmware. The address range
> > reported in the MCFG table or by _CBA method (see Section 4.1.3) must be
> > reserved by declaring a motherboard resource.
> 
> We don't do this today.
> 
> > For most systems, the
> > motherboard resource would appear at the root of the ACPI namespace
> > (under \_SB) in a node with a _HID of EISAID (PNP0C02), and the
> > resources in this case should not be claimed in the root PCI bus’s _CRS.
> 
> Which I read as _in case it is at the root of the apci namespace_ it
> should not be claimed in PCI0._CRS.  Which makes sense.
> 
> My laptop has it reserved in a \_SB\PCI0\LPC\SIO device instead:
> 
>             Device (LPC)
>             {
>                 Name (_ADR, 0x001F0000)  // _ADR: Address
>                 Name (_S3D, 0x03)  // _S3D: S3 Device State
>                 Name (RID, 0x00)
>                 Device (SIO)
>                 {
>                     Name (_HID, EisaId ("PNP0C02"))
>                     Name (_UID, 0x00)  // _UID: Unique ID
>                     Name (SCRS, ResourceTemplate ()
>                     {
>                         [ ... ]
>                         Memory32Fixed (ReadWrite,
>                             0xF8000000,         // Address Base
>                             0x04000000,         // Address Length
>                             )
>                         [ ... ]
> 
> cheers,
>   Gerd
> 

We can try, but Igor tried to do something like this recently (
for IO resources) and windows guests kept crashing
unless he made holes in _CRS.

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Igor Mammedov]

On Mon, 20 Jan 2014 17:51:47 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jan 20, 2014 at 04:36:57PM +0100, Gerd Hoffmann wrote:
> > > 4.1.2.
> > >  MCFG Table Description
> > > 
> > > 
> > > ...
> > > 
> > > If the operating system does not natively comprehend reserving the MMCFG
> > > region, the MMCFG region must be reserved by firmware. The address range
> > > reported in the MCFG table or by _CBA method (see Section 4.1.3) must be
> > > reserved by declaring a motherboard resource.
> > 
> > We don't do this today.
> > 
> > > For most systems, the
> > > motherboard resource would appear at the root of the ACPI namespace
> > > (under \_SB) in a node with a _HID of EISAID (PNP0C02), and the
> > > resources in this case should not be claimed in the root PCI bus’s _CRS.
> > 
> > Which I read as _in case it is at the root of the apci namespace_ it
> > should not be claimed in PCI0._CRS.  Which makes sense.
> > 
> > My laptop has it reserved in a \_SB\PCI0\LPC\SIO device instead:
> > 
> >             Device (LPC)
> >             {
> >                 Name (_ADR, 0x001F0000)  // _ADR: Address
> >                 Name (_S3D, 0x03)  // _S3D: S3 Device State
> >                 Name (RID, 0x00)
> >                 Device (SIO)
> >                 {
> >                     Name (_HID, EisaId ("PNP0C0P2"))
> >                     Name (_UID, 0x00)  // _UID: Unique ID
> >                     Name (SCRS, ResourceTemplate ()
> >                     {
> >                         [ ... ]
> >                         Memory32Fixed (ReadWrite,
> >                             0xF8000000,         // Address Base
> >                             0x04000000,         // Address Length
> >                             )
> >                         [ ... ]
> > 
> > cheers,
> >   Gerd
> > 
> 
> We can try, but Igor tried to do something like this recently (
> for IO resources) and windows guests kept crashing
> unless he made holes in _CRS.
I've tried to consume ranges under piix_pm/lpc device,
there were no any indication that ranges were ever consumed.

I haven't tried to put PNP0C0P2 on PCI bus though. It might work
I just found similar code in coreboot
https://www.mail-archive.com/coreboot@coreboot.org/msg27723.html

Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram

[Gerd Hoffmann]

> > > My laptop has it reserved in a \_SB\PCI0\LPC\SIO device instead:
> > > 
> > >             Device (LPC)
> > >             {
> > >                 Name (_ADR, 0x001F0000)  // _ADR: Address
> > >                 Name (_S3D, 0x03)  // _S3D: S3 Device State
> > >                 Name (RID, 0x00)
> > >                 Device (SIO)
> > >                 {
> > >                     Name (_HID, EisaId ("PNP0C0P2"))
> > >                     Name (_UID, 0x00)  // _UID: Unique ID
> > >                     Name (SCRS, ResourceTemplate ()
> > >                     {
> > >                         [ ... ]
> > >                         Memory32Fixed (ReadWrite,
> > >                             0xF8000000,         // Address Base
> > >                             0x04000000,         // Address Length
> > >                             )
> > >                         [ ... ]
> > > 
> > > cheers,
> > >   Gerd
> > > 
> > 
> > We can try, but Igor tried to do something like this recently (
> > for IO resources) and windows guests kept crashing
> > unless he made holes in _CRS.
> I've tried to consume ranges under piix_pm/lpc device,
> there were no any indication that ranges were ever consumed.

I see this in dmesg:

[    0.128636] PCI: MMCONFIG at [mem 0xf8000000-0xfbffffff] reserved in
ACPI motherboard resources

cheers,
  Gerd