Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Igor Mammedov
On Wed, 4 Sep 2013 13:48:29 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 For Q35, MMCFG address and size are guest configurable.
 Update w32 property to make it behave accordingly.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/pci-host/q35.c | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
 index 4febd24..3f1d447 100644
 --- a/hw/pci-host/q35.c
 +++ b/hw/pci-host/q35.c
 @@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
  }
  addr = pciexbar  addr_mask;
  pcie_host_mmcfg_update(pehb, enable, addr, length);
 +/* Leave enough space for the MCFG BAR */
 +/*
 + * TODO: this matches current bios behaviour, but it's not a power of 
 two,
 + * which means an MTRR can't cover it exactly.
 + */
 +if (enable) {
 +mch-pci_info.w32.begin = addr + length;
 +} else {
 +mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
 +}
  }
I probably miss something but where is remapping in system address space?
If there is none then, then updated w32 might mismatch actually/initially 
mapped alias.

  /* PAM */




Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Michael S. Tsirkin
On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
 On Wed, 4 Sep 2013 13:48:29 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  For Q35, MMCFG address and size are guest configurable.
  Update w32 property to make it behave accordingly.
 
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
   hw/pci-host/q35.c | 10 ++
   1 file changed, 10 insertions(+)
  
  diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
  index 4febd24..3f1d447 100644
  --- a/hw/pci-host/q35.c
  +++ b/hw/pci-host/q35.c
  @@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
   }
   addr = pciexbar  addr_mask;
   pcie_host_mmcfg_update(pehb, enable, addr, length);
  +/* Leave enough space for the MCFG BAR */
  +/*
  + * TODO: this matches current bios behaviour, but it's not a power of 
  two,
  + * which means an MTRR can't cover it exactly.
  + */
  +if (enable) {
  +mch-pci_info.w32.begin = addr + length;
  +} else {
  +mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
  +}
   }
 I probably miss something but where is remapping in system address space?
 If there is none then, then updated w32 might mismatch actually/initially 
 mapped alias.
 
   /* PAM */

You mean mmcfg?
The re-mapping is in hw/pci/pcie_host.c





Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Igor Mammedov
On Tue, 10 Sep 2013 16:46:36 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
  On Wed, 4 Sep 2013 13:48:29 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
  
   For Q35, MMCFG address and size are guest configurable.
   Update w32 property to make it behave accordingly.
  
   Signed-off-by: Michael S. Tsirkin m...@redhat.com
   ---
hw/pci-host/q35.c | 10 ++
1 file changed, 10 insertions(+)
   
   diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
   index 4febd24..3f1d447 100644
   --- a/hw/pci-host/q35.c
   +++ b/hw/pci-host/q35.c
   @@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
}
addr = pciexbar  addr_mask;
pcie_host_mmcfg_update(pehb, enable, addr, length);
   +/* Leave enough space for the MCFG BAR */
   +/*
   + * TODO: this matches current bios behaviour, but it's not a power 
   of two,
   + * which means an MTRR can't cover it exactly.
   + */
   +if (enable) {
   +mch-pci_info.w32.begin = addr + length;
   +} else {
   +mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
   +}
}
  I probably miss something but where is remapping in system address space?
  If there is none then, then updated w32 might mismatch actually/initially 
  mapped alias.
  
/* PAM */
 
 You mean mmcfg?
 The re-mapping is in hw/pci/pcie_host.c
no, I mean 32-bit PCI hole




Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Michael S. Tsirkin
On Tue, Sep 10, 2013 at 03:46:13PM +0200, Igor Mammedov wrote:
 On Tue, 10 Sep 2013 16:46:36 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
   On Wed, 4 Sep 2013 13:48:29 +0300
   Michael S. Tsirkin m...@redhat.com wrote:
   
For Q35, MMCFG address and size are guest configurable.
Update w32 property to make it behave accordingly.
   
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/pci-host/q35.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 4febd24..3f1d447 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
 }
 addr = pciexbar  addr_mask;
 pcie_host_mmcfg_update(pehb, enable, addr, length);
+/* Leave enough space for the MCFG BAR */
+/*
+ * TODO: this matches current bios behaviour, but it's not a power 
of two,
+ * which means an MTRR can't cover it exactly.
+ */
+if (enable) {
+mch-pci_info.w32.begin = addr + length;
+} else {
+mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
+}
 }
   I probably miss something but where is remapping in system address space?
   If there is none then, then updated w32 might mismatch actually/initially 
   mapped alias.
   
 /* PAM */
  
  You mean mmcfg?
  The re-mapping is in hw/pci/pcie_host.c
 no, I mean 32-bit PCI hole

It works differently - see mch_init.
32-bit PCI hole is not remapped - instead mmcfg overlaps it
and shadows a chunk of it.




Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Michael S. Tsirkin
On Tue, Sep 10, 2013 at 04:09:21PM +0200, Igor Mammedov wrote:
 On Tue, 10 Sep 2013 16:53:58 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Sep 10, 2013 at 03:46:13PM +0200, Igor Mammedov wrote:
   On Tue, 10 Sep 2013 16:46:36 +0300
   Michael S. Tsirkin m...@redhat.com wrote:
   
On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
 On Wed, 4 Sep 2013 13:48:29 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  For Q35, MMCFG address and size are guest configurable.
  Update w32 property to make it behave accordingly.
 
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
   hw/pci-host/q35.c | 10 ++
   1 file changed, 10 insertions(+)
  
  diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
  index 4febd24..3f1d447 100644
  --- a/hw/pci-host/q35.c
  +++ b/hw/pci-host/q35.c
  @@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState 
  *mch)
   }
   addr = pciexbar  addr_mask;
   pcie_host_mmcfg_update(pehb, enable, addr, length);
  +/* Leave enough space for the MCFG BAR */
  +/*
  + * TODO: this matches current bios behaviour, but it's not a 
  power of two,
  + * which means an MTRR can't cover it exactly.
  + */
  +if (enable) {
  +mch-pci_info.w32.begin = addr + length;
  +} else {
  +mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
  +}
   }
 I probably miss something but where is remapping in system address 
 space?
 If there is none then, then updated w32 might mismatch 
 actually/initially mapped alias.
 
   /* PAM */

You mean mmcfg?
The re-mapping is in hw/pci/pcie_host.c
   no, I mean 32-bit PCI hole
  
  It works differently - see mch_init.
  32-bit PCI hole is not remapped - instead mmcfg overlaps it
  and shadows a chunk of it.
  
 it probably won't cause harm, but actual pci_hole alias doesn't match
 its window reported via properties anymore (it's just inconsistent).

That's exactly what happens at the moment too:
for q35 pci hole has a chunk that is not reported in w32.

Do you imply that w32 should just report the PCI hole,
and make users (e.g. acpi) take mmcfg out of that?


-- 
MST



Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Igor Mammedov
On Tue, 10 Sep 2013 16:53:58 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Sep 10, 2013 at 03:46:13PM +0200, Igor Mammedov wrote:
  On Tue, 10 Sep 2013 16:46:36 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
On Wed, 4 Sep 2013 13:48:29 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 For Q35, MMCFG address and size are guest configurable.
 Update w32 property to make it behave accordingly.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/pci-host/q35.c | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
 index 4febd24..3f1d447 100644
 --- a/hw/pci-host/q35.c
 +++ b/hw/pci-host/q35.c
 @@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
  }
  addr = pciexbar  addr_mask;
  pcie_host_mmcfg_update(pehb, enable, addr, length);
 +/* Leave enough space for the MCFG BAR */
 +/*
 + * TODO: this matches current bios behaviour, but it's not a 
 power of two,
 + * which means an MTRR can't cover it exactly.
 + */
 +if (enable) {
 +mch-pci_info.w32.begin = addr + length;
 +} else {
 +mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
 +}
  }
I probably miss something but where is remapping in system address 
space?
If there is none then, then updated w32 might mismatch 
actually/initially mapped alias.

  /* PAM */
   
   You mean mmcfg?
   The re-mapping is in hw/pci/pcie_host.c
  no, I mean 32-bit PCI hole
 
 It works differently - see mch_init.
 32-bit PCI hole is not remapped - instead mmcfg overlaps it
 and shadows a chunk of it.
 
it probably won't cause harm, but actual pci_hole alias doesn't match
its window reported via properties anymore (it's just inconsistent).




Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Michael S. Tsirkin
On Tue, Sep 10, 2013 at 05:05:43PM +0200, Igor Mammedov wrote:
 On Tue, 10 Sep 2013 17:16:24 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Sep 10, 2013 at 04:09:21PM +0200, Igor Mammedov wrote:
   On Tue, 10 Sep 2013 16:53:58 +0300
   Michael S. Tsirkin m...@redhat.com wrote:
   
On Tue, Sep 10, 2013 at 03:46:13PM +0200, Igor Mammedov wrote:
 On Tue, 10 Sep 2013 16:46:36 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
   On Wed, 4 Sep 2013 13:48:29 +0300
   Michael S. Tsirkin m...@redhat.com wrote:
   
For Q35, MMCFG address and size are guest configurable.
Update w32 property to make it behave accordingly.
   
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/pci-host/q35.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 4febd24..3f1d447 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -214,6 +214,16 @@ static void 
mch_update_pciexbar(MCHPCIState *mch)
 }
 addr = pciexbar  addr_mask;
 pcie_host_mmcfg_update(pehb, enable, addr, length);
+/* Leave enough space for the MCFG BAR */
+/*
+ * TODO: this matches current bios behaviour, but it's not 
a power of two,
+ * which means an MTRR can't cover it exactly.
+ */
+if (enable) {
+mch-pci_info.w32.begin = addr + length;
+} else {
+mch-pci_info.w32.begin = 
MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
+}
 }
   I probably miss something but where is remapping in system 
   address space?
   If there is none then, then updated w32 might mismatch 
   actually/initially mapped alias.
   
 /* PAM */
  
  You mean mmcfg?
  The re-mapping is in hw/pci/pcie_host.c
 no, I mean 32-bit PCI hole

It works differently - see mch_init.
32-bit PCI hole is not remapped - instead mmcfg overlaps it
and shadows a chunk of it.

   it probably won't cause harm, but actual pci_hole alias doesn't match
   its window reported via properties anymore (it's just inconsistent).
  
  That's exactly what happens at the moment too:
  for q35 pci hole has a chunk that is not reported in w32.
  
  Do you imply that w32 should just report the PCI hole,
  and make users (e.g. acpi) take mmcfg out of that?

 initial pci_hole alias in system AS doesn't include mmcfg, it's mapped right 
 after mmcfg and
 w32 range matches alias exactly.

That would be a bug (unrelated to this patch series).
But I don't see this code: looking at mch_init,
memory_region_add_subregion(mch-system_memory, mch-below_4g_mem_size,
mch-pci_hole);

so we have pci hole right after memory, and overlapping mmcfg,
which makes sense to me.

 But after mmcfg update that might be not true. I suggest to
 move/remap alias to match new w32, to be consistent.



  
  



Re: [Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-10 Thread Igor Mammedov
On Tue, 10 Sep 2013 17:16:24 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Sep 10, 2013 at 04:09:21PM +0200, Igor Mammedov wrote:
  On Tue, 10 Sep 2013 16:53:58 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Tue, Sep 10, 2013 at 03:46:13PM +0200, Igor Mammedov wrote:
On Tue, 10 Sep 2013 16:46:36 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Sep 10, 2013 at 03:37:12PM +0200, Igor Mammedov wrote:
  On Wed, 4 Sep 2013 13:48:29 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
  
   For Q35, MMCFG address and size are guest configurable.
   Update w32 property to make it behave accordingly.
  
   Signed-off-by: Michael S. Tsirkin m...@redhat.com
   ---
hw/pci-host/q35.c | 10 ++
1 file changed, 10 insertions(+)
   
   diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
   index 4febd24..3f1d447 100644
   --- a/hw/pci-host/q35.c
   +++ b/hw/pci-host/q35.c
   @@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState 
   *mch)
}
addr = pciexbar  addr_mask;
pcie_host_mmcfg_update(pehb, enable, addr, length);
   +/* Leave enough space for the MCFG BAR */
   +/*
   + * TODO: this matches current bios behaviour, but it's not a 
   power of two,
   + * which means an MTRR can't cover it exactly.
   + */
   +if (enable) {
   +mch-pci_info.w32.begin = addr + length;
   +} else {
   +mch-pci_info.w32.begin = 
   MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
   +}
}
  I probably miss something but where is remapping in system address 
  space?
  If there is none then, then updated w32 might mismatch 
  actually/initially mapped alias.
  
/* PAM */
 
 You mean mmcfg?
 The re-mapping is in hw/pci/pcie_host.c
no, I mean 32-bit PCI hole
   
   It works differently - see mch_init.
   32-bit PCI hole is not remapped - instead mmcfg overlaps it
   and shadows a chunk of it.
   
  it probably won't cause harm, but actual pci_hole alias doesn't match
  its window reported via properties anymore (it's just inconsistent).
 
 That's exactly what happens at the moment too:
 for q35 pci hole has a chunk that is not reported in w32.
 
 Do you imply that w32 should just report the PCI hole,
 and make users (e.g. acpi) take mmcfg out of that?
initial pci_hole alias in system AS doesn't include mmcfg, it's mapped right 
after mmcfg and
w32 range matches alias exactly. But after mmcfg update that might be not true. 
I suggest to
move/remap alias to match new w32, to be consistent.

 
 




[Qemu-devel] [PATCH 1/6] q35: make pci window address/size match guest cfg

2013-09-04 Thread Michael S. Tsirkin
For Q35, MMCFG address and size are guest configurable.
Update w32 property to make it behave accordingly.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/pci-host/q35.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 4febd24..3f1d447 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -214,6 +214,16 @@ static void mch_update_pciexbar(MCHPCIState *mch)
 }
 addr = pciexbar  addr_mask;
 pcie_host_mmcfg_update(pehb, enable, addr, length);
+/* Leave enough space for the MCFG BAR */
+/*
+ * TODO: this matches current bios behaviour, but it's not a power of two,
+ * which means an MTRR can't cover it exactly.
+ */
+if (enable) {
+mch-pci_info.w32.begin = addr + length;
+} else {
+mch-pci_info.w32.begin = MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT;
+}
 }
 
 /* PAM */
-- 
MST