date:20201229

On 12/29/20 6:26 AM, Jiaxun Yang wrote:
> 在 2020/12/29 上午11:26, BALATON Zoltan 写道:
>> Hello,
>>
>> While continuing with part two of my vt82c686b clean ups I've tried to
>> implement SMBus IO base configuration in the vt82c686b-pm part that
>> I've already done for vt8231 for pegasos2 and it should be the same
>> for 686B. (In short, writing address to pm config 0x90 sets base
>> address of smbus regs and bit 0 of 0xd2 enables/disables it.) This is
>> what the firmware does first and it would allow removing hard coded
>> 0xeee1 value and the property to set it and then I could reuse the
>> same PM part in VT8231.
>>
> [...]
>>
>> Any idea what this is trying to do and how to fix it?
> 
> It's trying to translate Bonito style PCI config space r/w to standard PCI
> config space R/W.
> 
> A quick galance told me change BONITO_PCICONF_REG_MASK to 0xff
> may help.

Per the datasheet section "5.7.5. Accessing PCI configuration space"
0xfc is the correct value, but the register number starts at the 2nd
bit. So this is not a write access to register 0xd2 but 0x34?

If you can test, this is the snippet I plan to send later:

-- >8 --
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index a99eced0657..65953766dd0 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -189,3 +189,3 @@ FIELD(BONGENCFG, PCIQUEUE,  12, 1)
 #define BONITO_PCICONF_REG_MASK0xFC
-#define BONITO_PCICONF_REG_OFFSET  0
+#define BONITO_PCICONF_REG_OFFSET  2
---

> 
> Thanks.
> 
> - Jiaxun
> 
>>
>> Regards,
>> BALATON Zoltan
> 
>

Re: [PATCH v2 1/2] ide: Make room for flags in PCIIDEState and add one for legacy mode

On 12/28/20 8:30 PM, Mark Cave-Ayland wrote:
> On 27/12/2020 22:13, BALATON Zoltan wrote:
> 
>> We'll need a flag for implementing some device specific behaviour in
>> via-ide but we already have a currently CMD646 specific field that can
>> be repurposed for this and leave room for further flags if needed in
>> the future. This patch changes the "secondary" field to "flags" and
>> change CMD646 and its users accordingly and define a new flag for
>> forcing legacy mode that will be used by via-ide for now.
>>
>> Signed-off-by: BALATON Zoltan 
>> Reviewed-by: Philippe Mathieu-Daudé 
>> Reviewed-by: Guenter Roeck 
>> Tested-by: Guenter Roeck 
>> ---
>> v2: Fixed typo in commit message
>>
>>   hw/ide/cmd646.c  | 4 ++--
>>   hw/sparc64/sun4u.c   | 2 +-
>>   include/hw/ide/pci.h | 7 ++-
>>   3 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
>> index c254631485..7a96016116 100644
>> --- a/hw/ide/cmd646.c
>> +++ b/hw/ide/cmd646.c
>> @@ -256,7 +256,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev,
>> Error **errp)
>>   pci_conf[PCI_CLASS_PROG] = 0x8f;
>>     pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
> 
> Doesn't the existing comment above cause checkpatch to fail?

The comment is old and Balaton didn't modified it. Usually I'd prefer
to address style change in a separate commit, ...

> 
>> -    if (d->secondary) {
>> +    if (d->flags & BIT(PCI_IDE_SECONDARY)) {
>>   /* XXX: if not enabled, really disable the seconday IDE
>> controller */
>>   pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */

... but since a similar comment is added here, it might be acceptable
to fix the style of the former one here too. I noted Balaton already
addressed your comment in a v3.

>>   }
>> @@ -314,7 +314,7 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
>>   }
>>     static Property cmd646_ide_properties[] = {
>> -    DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
>> +    DEFINE_PROP_BIT("secondary", PCIIDEState, flags,
>> PCI_IDE_SECONDARY, false),
>>   DEFINE_PROP_END_OF_LIST(),
>>   };

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support

On 12/28/20 9:50 PM, BALATON Zoltan via wrote:
> On Mon, 28 Dec 2020, Mark Cave-Ayland wrote:
>> On 27/12/2020 22:13, BALATON Zoltan via wrote:
>>
>>> From: Guenter Roeck 
>>>
>>> The IDE legacy mode emulation has been removed in commit 4ea98d317eb
>>> ("ide/via: Implement and use native PCI IDE mode") but some Linux
>>> kernels (probably including def_config) require legacy mode on the
>>> Fuloong2e so only emulating native mode did not turn out feasible.
>>> Add property to via-ide model to make the mode configurable, and set
>>> legacy mode for Fuloong2e.
>>>
>>> Signed-off-by: Guenter Roeck 
>>> [balaton: Use bit in flags for property, add comment for missing BAR4]
>>> Signed-off-by: BALATON Zoltan 
>>> Reviewed-by: Philippe Mathieu-Daudé 
>>> Tested-by: Guenter Roeck 
>>> ---
>>> v2: Reworded commit message
>>>
>>>   hw/ide/via.c    | 19 +--
>>>   hw/mips/fuloong2e.c |  4 +++-
>>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/ide/via.c b/hw/ide/via.c
>>> index be09912b33..7d54d7e829 100644
>>> --- a/hw/ide/via.c
>>> +++ b/hw/ide/via.c
>>> @@ -26,6 +26,7 @@
>>>     #include "qemu/osdep.h"
>>>   #include "hw/pci/pci.h"
>>> +#include "hw/qdev-properties.h"
>>>   #include "migration/vmstate.h"
>>>   #include "qemu/module.h"
>>>   #include "sysemu/dma.h"
>>> @@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev,
>>> Error **errp)
>>>     &d->bus[1], "via-ide1-cmd", 4);
>>>   pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO,
>>> &d->cmd_bar[1]);
>>>   -    bmdma_setup_bar(d);
>>> -    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
>>> +    if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
>>> +    /* Missing BAR4 will make Linux driver fall back to legacy
>>> PIO mode */
>>> +    bmdma_setup_bar(d);
>>> +    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO,
>>> &d->bmdma_bar);
>>> +    }
>>
>> Since the default value of the legacy mode parameter is false, then
>> this means the device assumes native mode by default. Therefore
>> PCI_CLASS_PROG should be set to 0x8f unless legacy mode is being used,
>> in which case it should be 0x8a.
> 
> I think this casued problems before because if it's not set to 0x8a
> (legacy) at start then guests may assume it's already switched to native
> mode by firmware and won't program the BARs and it will not work. This
> way, even if it looks odd all guests I've tested work so I don't want to
> touch this, because I don't want to test all guests again.

If you can describe on the list how you do your testing (mostly
command line used, where image/demo can be downloaded), we might help
writing an integration test to automate the testing. Don't worry if
it involves using close-source binaries, we'll try to figure out a
way.

Regards,

Phil.

Re: Bug in Bonito? (mips/fuloong2e)


On Tue, 29 Dec 2020, Philippe Mathieu-Daudé wrote:

On 12/29/20 6:26 AM, Jiaxun Yang wrote:

在 2020/12/29 上午11:26, BALATON Zoltan 写道:

Hello,

While continuing with part two of my vt82c686b clean ups I've tried to
implement SMBus IO base configuration in the vt82c686b-pm part that
I've already done for vt8231 for pegasos2 and it should be the same
for 686B. (In short, writing address to pm config 0x90 sets base
address of smbus regs and bit 0 of 0xd2 enables/disables it.) This is
what the firmware does first and it would allow removing hard coded
0xeee1 value and the property to set it and then I could reuse the
same PM part in VT8231.


[...]


Any idea what this is trying to do and how to fix it?


It's trying to translate Bonito style PCI config space r/w to standard PCI
config space R/W.

A quick galance told me change BONITO_PCICONF_REG_MASK to 0xff
may help.


With Jiaxun's change it works better:

bonito_spciconf_write: bonito_spciconf_write 0490 size 4 val eee1
bonito_sbridge_pciaddr: cfgaddr 10490 pciaddr 2c90 busno 0 devno 5 funno 4 
regno 144
pci_cfg_write vt82c686b-pm 05:4 @0x90 <- 0xeee1
via_pm_write addr 0x90 val 0xeee1 len 0x4

bonito_writel: bonito_writel 0018 val 1 saddr 6
bonito_readl: bonito_readl 0018

bonito_spciconf_write: bonito_spciconf_write 04d2 size 2 val 1
bonito_sbridge_pciaddr: cfgaddr 104d2 pciaddr 2cd2 busno 0 devno 5 funno 4 
regno 210
pci_cfg_write vt82c686b-pm 05:4 @0xd2 <- 0x1
via_pm_write addr 0xd2 val 0x1 len 0x2
*** Should update smbus io base to eee1, enable

(last line is my debug message). Not sure if this breaks anyhing else but 
PMON still seems to work (tested with pmon_2e.bin).



Per the datasheet section "5.7.5. Accessing PCI configuration space"
0xfc is the correct value, but the register number starts at the 2nd
bit. So this is not a write access to register 0xd2 but 0x34?

If you can test, this is the snippet I plan to send later:

-- >8 --
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index a99eced0657..65953766dd0 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -189,3 +189,3 @@ FIELD(BONGENCFG, PCIQUEUE,  12, 1)
#define BONITO_PCICONF_REG_MASK0xFC
-#define BONITO_PCICONF_REG_OFFSET  0
+#define BONITO_PCICONF_REG_OFFSET  2
---


This does not seem to work, I get all mixed up regs:

bonito_spciconf_write: bonito_spciconf_write 0490 size 4 val eee1
bonito_sbridge_pciaddr: cfgaddr 10490 pciaddr 2c24 busno 0 devno 5 funno 4 
regno 36
pci_cfg_write vt82c686b-pm 05:4 @0x24 <- 0xeee1
via_pm_write addr 0x24 val 0xeee1 len 0x4

bonito_writel: bonito_writel 0018 val 1 saddr 6
bonito_readl: bonito_readl 0018

bonito_spciconf_write: bonito_spciconf_write 04d2 size 2 val 1
bonito_sbridge_pciaddr: cfgaddr 104d2 pciaddr 2c34 busno 0 devno 5 funno 4 
regno 52
pci_cfg_write vt82c686b-pm 05:4 @0x34 <- 0x1
via_pm_write addr 0x34 val 0x1 len 0x2

So I'll use the first one for now, that allows me to continue with vt82c686b.

Regards,
BALATON Zoltan

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support


On 28/12/2020 20:50, BALATON Zoltan via wrote:


diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..7d54d7e829 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
    #include "qemu/osdep.h"
  #include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
  #include "migration/vmstate.h"
  #include "qemu/module.h"
  #include "sysemu/dma.h"
@@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
    &d->bus[1], "via-ide1-cmd", 4);
  pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
  -    bmdma_setup_bar(d);
-    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+    if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
+    /* Missing BAR4 will make Linux driver fall back to legacy PIO mode */
+    bmdma_setup_bar(d);
+    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+    }


Since the default value of the legacy mode parameter is false, then this means the 
device assumes native mode by default. Therefore PCI_CLASS_PROG should be set to 
0x8f unless legacy mode is being used, in which case it should be 0x8a.


I think this casued problems before because if it's not set to 0x8a (legacy) at start 
then guests may assume it's already switched to native mode by firmware and won't 
program the BARs and it will not work. This way, even if it looks odd all guests I've 
tested work so I don't want to touch this, because I don't want to test all guests 
again.


Keep in mind we're not fully emulating this device so not every combination that may 
work on real hardware work in this model. We really either only emulate "half-native" 
mode (i.e. native mode with ISA IRQs) that's needed for pegasos2 guests or now again 
only emulate legacy mode if property is set for Linux on fuloong2e. (My original 
patch emulated half-native and native mode instead of legacy thinking that Linux on 
fuloong2e would adapt.) All other combinations, including switching between these two 
is expected to not work which is due to QEMU limitations as you've also discovered 
now. I think this is still an improvement over only emulating legacy mode before even 
if it does not yet fully model the chip and I've also cleaned up PCI IDE emulation 
during implementing this so that code can be shared between via-ide, sii3112 and 
CMD646 without much duplication.



  qdev_init_gpio_in(ds, via_ide_set_irq, 2);
  for (i = 0; i < 2; i++) {
  ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+    if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
+    ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0,
+    i ? 0x376 : 0x3f6);
+    }


You could actually remove the if() here: PCI configuration always leaves a gap at 
the lower end of IO address space to avoid clashes with legacy ports. Therefore if 
a guest decides to switch to native mode and configure the BAR, it will never clash 
with the default legacy IDE ports. This has the advantage of minimising the parts 
protected by PCI_IDE_LEGACY_MODE whilst also providing the legacy ports if someone 
can devise a method to dynamically switch between compatible and native modes later.


I think leaving the legacy ports enabled is a bad idea for at least two reasons: 1) 
It may clash with other io ports on other machines, e.g. I'm not sure on PPC where 
firmware or OS does not expect to see legacy ISA ports won't map some io BAR of a PCI 
card there. 2) If this is left enabled there would be two ports poking the same 
registers so if that does not cause a problem by itself, writing to one accidentally 
(like when something is mapped over it) could cause corruption of IDE state so I 
think it's much better to protect this than later trying to debug such problems. (You 
can't get rid of the flag without implementing mode switching that needs rewrite of 
ISA and PCI emulation in QEMU so just get over it.)


Okay I'm getting confused now: I thought the original aim of this patchset was to 
allow a guest OS to switch VIA IDE to compatible mode by default? Then again 
whichever way you look at this, regardless of whether PCI_CLASS_PROG is set to 0x8a 
or 0x8f then QEMU's VIA IDE device is still advertising unsupported configurations to 
the guest OS.


0x8a means "I am currently in compatible mode but I can switch to native mode" so if 
this is the default then by definition the legacy IDE ioports *MUST* be accessible by 
the guest. Otherwise a guest OS will read PCI_CLASS_PROG and determine that the VIA 
IDE is in legacy mode, try to read the legacy IDE ioport and break which is what is 
happening now.


0x8f means "I am currently in native mode" but this is also a lie to the guest since 
whilst the BARs exist and can now be programmed thanks to your earlier patches, there 
is no PCI IRQ routing (i.e. no call to pci_set_irq()).


Fortunately with PCI_CLASS_PROG at 0x8a Linux will keep the VIA IDE in compatible 
mode and not attempt t

Re: [PATCH v2] acpi: Permit OEM ID and OEM table ID fields to be changed

On Wed, 23 Dec 2020 23:56:30 +0200
Marian Posteuca  wrote:

> Thanks for the thorough review.
> 
> Igor Mammedov  writes:
> > On Tue, 22 Dec 2020 13:33:53 +0200
> > Marian Posteuca  wrote:
> >
> > I see defaults are now initialized in pcmc->oem_[table_]id fields,
> > and sometimes used from there, so question is why
> > do we need use_sig_oem and keeping old code
> >
> > if (oem_id) {   
> >  
> > strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);   
> >  
> > } else {
> >  
> > memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);  
> >  
> > }   
> >  
> > 
> >  
> > if ()) {  
> > strncpy((char *)h->oem_table_id, oem_table_id, 
> > sizeof(h->oem_table_id)); 
> > } else {
> >  
> > memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
> >  
> > memcpy(h->oem_table_id + 4, sig, 4);
> >  
> > }  
> > I'd rather drop 'else' branches altogether and simplify to something like 
> > this
> >
> > g_assert(oem_id);
> > strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> > g_assert(oem_table_id)
> > strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h->oem_table_id));
> > + padding
> >
> > and make sure ids are properly propagated everywhere.
> >  
> 
> I'm not sure if I understood this point correctly. You want to remove the 
> appending
> of the sig part to the oem_table_id field, and just use whatever is
> passed by the caller for oem_table_id?
yes, according to spec unique oem_table_id helps only in distinguishing 
different
pieces of DSDT/SSDT tables, for other tables it doesn't make any sense to make 
it unique.
and this matches what real machines do.

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support

On Tue, 29 Dec 2020, Philippe Mathieu-Daudé wrote:

I think this casued problems before because if it's not set to 0x8a
(legacy) at start then guests may assume it's already switched to native
mode by firmware and won't program the BARs and it will not work. This
way, even if it looks odd all guests I've tested work so I don't want to
touch this, because I don't want to test all guests again.

If you can describe on the list how you do your testing (mostly
command line used, where image/demo can be downloaded), we might help
writing an integration test to automate the testing. Don't worry if
it involves using close-source binaries, we'll try to figure out a
way.

It's documented here:

https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2#h2-Current.20status.20and.20outstanding.20issues

but pegasos2 is not in QEMU master yet, I'm just trying to clean it up and
submit it so testing likely comes after I have a set of patches to send to
add pegasos2. The version in qmiga repo is not the latest but one that
works in itself, my current vt82c686c work is to upstream the vt8231 part.
After that the rest should be simple: add mv64361 model and pegasos2 board
code. The only problem I see with those is if it's OK to have board
needing ROM image or if Mark torpedoes it for some reason :-). For the ROM
I have plans to use vof (virtual open firmware emulation within QEMU,
unmerged on qemu-ppc list currently for a while) but I couldn't get to
that yet. Maybe Linux can be used via -kernel but I think it also needs
Open Firmware client interface on pegasos2 so it won't work without some
kind of firmware. (All this is also documented on above page if you scroll
up.)

(By the way I've created the qmiga.osdn.net project with the intent to
share work with interested people on these but so far nobody seemed to be
interested enough to join and help. There were about 2 or 3 people
interested in ati-vga but got scared away when saw how much work is needed
yet. Contributions are still welcome otherwise it will take another few
years to finish.)

Regards,
BALATON Zoltan

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support


On 28/12/2020 20:50, BALATON Zoltan via wrote:

I think leaving the legacy ports enabled is a bad idea for at least two reasons: 1) 
It may clash with other io ports on other machines, e.g. I'm not sure on PPC where 
firmware or OS does not expect to see legacy ISA ports won't map some io BAR of a PCI 
card there. 2) If this is left enabled there would be two ports poking the same 
registers so if that does not cause a problem by itself, writing to one accidentally 
(like when something is mapped over it) could cause corruption of IDE state so I 
think it's much better to protect this than later trying to debug such problems.


Legacy ioports originate in the x86 world, however all the PCI bus enumeration code 
I've seen reserves the lower part of the IO address space to prevent such problems 
with e.g. a BIOS starting up in legacy mode.



ATB,

Mark.

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support


On Tue, 29 Dec 2020, Mark Cave-Ayland wrote:

On 28/12/2020 20:50, BALATON Zoltan via wrote:

diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..7d54d7e829 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
    #include "qemu/osdep.h"
  #include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
  #include "migration/vmstate.h"
  #include "qemu/module.h"
  #include "sysemu/dma.h"
@@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error 
**errp)

    &d->bus[1], "via-ide1-cmd", 4);
  pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, 
&d->cmd_bar[1]);

  -    bmdma_setup_bar(d);
-    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);
+    if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
+    /* Missing BAR4 will make Linux driver fall back to legacy PIO 
mode */

+    bmdma_setup_bar(d);
+    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, 
&d->bmdma_bar);

+    }


Since the default value of the legacy mode parameter is false, then this 
means the device assumes native mode by default. Therefore PCI_CLASS_PROG 
should be set to 0x8f unless legacy mode is being used, in which case it 
should be 0x8a.


I think this casued problems before because if it's not set to 0x8a 
(legacy) at start then guests may assume it's already switched to native 
mode by firmware and won't program the BARs and it will not work. This way, 
even if it looks odd all guests I've tested work so I don't want to touch 
this, because I don't want to test all guests again.


Keep in mind we're not fully emulating this device so not every combination 
that may work on real hardware work in this model. We really either only 
emulate "half-native" mode (i.e. native mode with ISA IRQs) that's needed 
for pegasos2 guests or now again only emulate legacy mode if property is 
set for Linux on fuloong2e. (My original patch emulated half-native and 
native mode instead of legacy thinking that Linux on fuloong2e would 
adapt.) All other combinations, including switching between these two is 
expected to not work which is due to QEMU limitations as you've also 
discovered now. I think this is still an improvement over only emulating 
legacy mode before even if it does not yet fully model the chip and I've 
also cleaned up PCI IDE emulation during implementing this so that code can 
be shared between via-ide, sii3112 and CMD646 without much duplication.



  qdev_init_gpio_in(ds, via_ide_set_irq, 2);
  for (i = 0; i < 2; i++) {
  ide_bus_new(&d->bus[i], sizeof(d->bus[i]), ds, i, 2);
+    if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
+    ide_init_ioport(&d->bus[i], NULL, i ? 0x170 : 0x1f0,
+    i ? 0x376 : 0x3f6);
+    }


You could actually remove the if() here: PCI configuration always leaves a 
gap at the lower end of IO address space to avoid clashes with legacy 
ports. Therefore if a guest decides to switch to native mode and configure 
the BAR, it will never clash with the default legacy IDE ports. This has 
the advantage of minimising the parts protected by PCI_IDE_LEGACY_MODE 
whilst also providing the legacy ports if someone can devise a method to 
dynamically switch between compatible and native modes later.


I think leaving the legacy ports enabled is a bad idea for at least two 
reasons: 1) It may clash with other io ports on other machines, e.g. I'm 
not sure on PPC where firmware or OS does not expect to see legacy ISA 
ports won't map some io BAR of a PCI card there. 2) If this is left enabled 
there would be two ports poking the same registers so if that does not 
cause a problem by itself, writing to one accidentally (like when something 
is mapped over it) could cause corruption of IDE state so I think it's much 
better to protect this than later trying to debug such problems. (You can't 
get rid of the flag without implementing mode switching that needs rewrite 
of ISA and PCI emulation in QEMU so just get over it.)


Okay I'm getting confused now: I thought the original aim of this patchset 
was to allow a guest OS to switch VIA IDE to compatible mode by default? Then


No, switching is still not supported and as you also discovered it's not 
easy to implement in QEMU so aim is to have property to set if we want to 
emulate legacy mode only or (half) native mode only.


again whichever way you look at this, regardless of whether PCI_CLASS_PROG is 
set to 0x8a or 0x8f then QEMU's VIA IDE device is still advertising 
unsupported configurations to the guest OS.


Yes, we're not fully emulating this device (see above) but advertise 
configurations that work WITH THE GUESTS! Pegasos2 guests will read 
PCI_CLASS_PROG and then switch to native mode (but still expecting 
interrupts on IRQ 14/15) so nothing will actually try to use legacy mode 
there but if it's not in that mode at the start it may not correctly 
program native mode. This may confuse other guests but I don't care as 
long as th

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support


On Tue, 29 Dec 2020, Mark Cave-Ayland wrote:

On 28/12/2020 20:50, BALATON Zoltan via wrote:
I think leaving the legacy ports enabled is a bad idea for at least two 
reasons: 1) It may clash with other io ports on other machines, e.g. I'm 
not sure on PPC where firmware or OS does not expect to see legacy ISA 
ports won't map some io BAR of a PCI card there. 2) If this is left enabled 
there would be two ports poking the same registers so if that does not 
cause a problem by itself, writing to one accidentally (like when something 
is mapped over it) could cause corruption of IDE state so I think it's much 
better to protect this than later trying to debug such problems.


Legacy ioports originate in the x86 world, however all the PCI bus 
enumeration code I've seen reserves the lower part of the IO address space to 
prevent such problems with e.g. a BIOS starting up in legacy mode.


I don't remember the details and won't test it again but PPC has neither 
BIOS nor legacy io ports (or io ports for that matter, all that is memory 
mapped). If you want go back to the email thread in March where I've 
described in detail how I ended up with these and what are the assumptions 
of guests I've tested and tried to satisfy with this.


Stop trying to compare it with hardware and look at it in a way that we 
want to make a device model that works with the guests we want to run. In 
that frame this works and probably the simplest way. Unless you fully want 
to implement all the quirks of the chip and take up all the clean up in 
QEMU needed for that (possibly risking breaking a lot of other boards 
along the way) this won't match hardware 100%.


Regards,
BALATON Zoltan

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support


On 29/12/2020 12:01, BALATON Zoltan via wrote:

Fortunately with PCI_CLASS_PROG at 0x8a Linux will keep the VIA IDE in compatible 
mode and not attempt to switch to native mode: therefore if you keep this as-is and 
add the legacy IDE ioports back, that just leaves the problem with BAR4 (BMDMA). In 
effect your patch isn't setting compatible mode anymore, it is just disabling BMDMA.


It's actually Guenter's patch so you're now bashing him not me...


This is a deliberately misleading comment, and not a good introduction for someone 
external becoming involved with the project. Guenter's patch was a PoC demonstrating 
how to fix the fuloong2e machine, which is really appreciated since it clearly 
locates the problems to allow a fix to be applied upstream.


(But I also think your time could be better spent than getting rid of this hack when 
there are much more hacks or missing functionalities to get rid of in the part you 
maintain.)


And comments like this are not appropriate for a technical mailing list either. I've 
done my best to review your patch in good faith (including reading related 
specifications and testing your pegasos2 model) and explain why it isn't reporting 
the correct information to the guest.


Phil - I hope you find that found my review comments useful and that they explain why 
the patchset is wrong by always claiming legacy IDE ioports exist but not providing 
them unless the new option is set (and indeed indicating some of the shortcomings of 
QEMU related to PCI BARs in this area that can be improved in future). As I feel 
comments in this thread have become directed at me personally, I am going to take a 
step back from this.



ATB,

Mark.

Re: Bug in Bonito? (mips/fuloong2e)

2020-12-29 Thread Jiaxun Yang





在 2020/12/29 18:47, Philippe Mathieu-Daudé 写道:

On 12/29/20 6:26 AM, Jiaxun Yang wrote:

在 2020/12/29 上午11:26, BALATON Zoltan 写道:

Hello,

While continuing with part two of my vt82c686b clean ups I've tried to
implement SMBus IO base configuration in the vt82c686b-pm part that
I've already done for vt8231 for pegasos2 and it should be the same
for 686B. (In short, writing address to pm config 0x90 sets base
address of smbus regs and bit 0 of 0xd2 enables/disables it.) This is
what the firmware does first and it would allow removing hard coded
0xeee1 value and the property to set it and then I could reuse the
same PM part in VT8231.


[...]

Any idea what this is trying to do and how to fix it?

It's trying to translate Bonito style PCI config space r/w to standard PCI
config space R/W.

A quick galance told me change BONITO_PCICONF_REG_MASK to 0xff
may help.

Per the datasheet section "5.7.5. Accessing PCI configuration space"
0xfc is the correct value, but the register number starts at the 2nd
bit. So this is not a write access to register 0xd2 but 0x34?


It seems like Loongson changed defination of the register?
There is no shifting in kernel[1] as well.

```
/* Type 1 configuration for offboard PCI bus */
addr = (busnum << 16) | (device << 11) | (function << 8) | reg;
```

Thanks.

[1]: 
https://elixir.bootlin.com/linux/latest/source/arch/mips/pci/ops-loongson2.c


- Jiaxun


If you can test, this is the snippet I plan to send later:

-- >8 --
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index a99eced0657..65953766dd0 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -189,3 +189,3 @@ FIELD(BONGENCFG, PCIQUEUE,  12, 1)
  #define BONITO_PCICONF_REG_MASK0xFC
-#define BONITO_PCICONF_REG_OFFSET  0
+#define BONITO_PCICONF_REG_OFFSET  2
---


Thanks.

- Jiaxun


Regards,
BALATON Zoltan

Re: [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type

On Mon, 14 Dec 2020 15:50:26 -0500
Eduardo Habkost  wrote:

> This includes code previously submitted[1] by Daniel P. Berrangé
> to add a "smbios-ep" machine property on PC.
> 
> SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a
> large number of VCPUs can easily hit the table size limit of
> SMBIOS 2.1 entry points.

Eduardo,
do you plan to submit Seabios patches for SMBIOS 3.0?
will OVMF pick up new tables automatically?

> 
> [1] 
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com
> 
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berra...@redhat.com
> 
> Daniel P. Berrangé (1):
>   hw/i386: expose a "smbios-ep" PC machine property
> 
> Eduardo Habkost (2):
>   smbios: Rename SMBIOS_ENTRY_POINT_* enums
>   hw/smbios: Use qapi for SmbiosEntryPointType
> 
>  qapi/qapi-schema.json|  1 +
>  qapi/smbios.json | 11 +++
>  include/hw/firmware/smbios.h | 10 ++
>  include/hw/i386/pc.h |  3 +++
>  hw/arm/virt.c|  2 +-
>  hw/i386/pc.c | 26 ++
>  hw/i386/pc_piix.c|  2 +-
>  hw/i386/pc_q35.c |  2 +-
>  hw/smbios/smbios.c   |  8 
>  qapi/meson.build |  1 +
>  10 files changed, 51 insertions(+), 15 deletions(-)
>  create mode 100644 qapi/smbios.json
>

Re: [PATCH v2 1/3] smbios: Rename SMBIOS_ENTRY_POINT_* enums

On Mon, 14 Dec 2020 15:50:27 -0500
Eduardo Habkost  wrote:

> Rename the enums to match the naming style used by QAPI.  This
> will allow us to more easily move the enum to the QAPI schema
> later.
> 
> Based on portions of a patch submitted by Daniel P. Berrangé.
> 
> Signed-off-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: Igor Mammedov 

> ---
> First version of this code was submitted at:
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com
> 
> Changes from v1:
> * Patch was split in two
> * Hunks included this patch are not changed from v1
> ---
>  include/hw/firmware/smbios.h | 4 ++--
>  hw/arm/virt.c| 2 +-
>  hw/i386/pc_piix.c| 2 +-
>  hw/i386/pc_q35.c | 2 +-
>  hw/smbios/smbios.c   | 8 
>  5 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 02a0ced0a0..5467ecec78 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -27,8 +27,8 @@ struct smbios_phys_mem_area {
>   * SMBIOS spec defined tables
>   */
>  typedef enum SmbiosEntryPointType {
> -SMBIOS_ENTRY_POINT_21,
> -SMBIOS_ENTRY_POINT_30,
> +SMBIOS_ENTRY_POINT_TYPE_2_1,
> +SMBIOS_ENTRY_POINT_TYPE_3_0,
>  } SmbiosEntryPointType;
>  
>  /* SMBIOS Entry Point
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 556592012e..af53e09d1e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1445,7 +1445,7 @@ static void virt_build_smbios(VirtMachineState *vms)
>  
>  smbios_set_defaults("QEMU", product,
>  vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
> -true, SMBIOS_ENTRY_POINT_30);
> +true, SMBIOS_ENTRY_POINT_TYPE_3_0);
>  
>  smbios_get_tables(MACHINE(vms), NULL, 0, &smbios_tables, 
> &smbios_tables_len,
>&smbios_anchor, &smbios_anchor_len);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 6188c3e97e..08b82df4d1 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -179,7 +179,7 @@ static void pc_init1(MachineState *machine,
>  smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
>  mc->name, pcmc->smbios_legacy_mode,
>  pcmc->smbios_uuid_encoded,
> -SMBIOS_ENTRY_POINT_21);
> +SMBIOS_ENTRY_POINT_TYPE_2_1);
>  }
>  
>  /* allocate ram and load rom/bios */
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0a212443aa..f71b1e2dcf 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -198,7 +198,7 @@ static void pc_q35_init(MachineState *machine)
>  smbios_set_defaults("QEMU", "Standard PC (Q35 + ICH9, 2009)",
>  mc->name, pcmc->smbios_legacy_mode,
>  pcmc->smbios_uuid_encoded,
> -SMBIOS_ENTRY_POINT_21);
> +SMBIOS_ENTRY_POINT_TYPE_2_1);
>  }
>  
>  /* allocate ram and load rom/bios */
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index f22c4f5b73..930cf52c6b 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -61,7 +61,7 @@ uint8_t *smbios_tables;
>  size_t smbios_tables_len;
>  unsigned smbios_table_max;
>  unsigned smbios_table_cnt;
> -static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
> +static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_TYPE_2_1;
>  
>  static SmbiosEntryPoint ep;
>  
> @@ -383,7 +383,7 @@ static void smbios_validate_table(MachineState *ms)
>  exit(1);
>  }
>  
> -if (smbios_ep_type == SMBIOS_ENTRY_POINT_21 &&
> +if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_2_1 &&
>  smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) {
>  error_report("SMBIOS 2.1 table length %zu exceeds %d",
>   smbios_tables_len, SMBIOS_21_MAX_TABLES_LEN);
> @@ -831,7 +831,7 @@ void smbios_set_defaults(const char *manufacturer, const 
> char *product,
>  static void smbios_entry_point_setup(void)
>  {
>  switch (smbios_ep_type) {
> -case SMBIOS_ENTRY_POINT_21:
> +case SMBIOS_ENTRY_POINT_TYPE_2_1:
>  memcpy(ep.ep21.anchor_string, "_SM_", 4);
>  memcpy(ep.ep21.intermediate_anchor_string, "_DMI_", 5);
>  ep.ep21.length = sizeof(struct smbios_21_entry_point);
> @@ -854,7 +854,7 @@ static void smbios_entry_point_setup(void)
>  ep.ep21.structure_table_address = cpu_to_le32(0);
>  
>  break;
> -case SMBIOS_ENTRY_POINT_30:
> +case SMBIOS_ENTRY_POINT_TYPE_3_0:
>  memcpy(ep.ep30.anchor_string, "_SM3_", 5);
>  ep.ep30.length = sizeof(struct smbios_30_entry_point);
>  ep.ep30.entry_point_revision = 1;

Re: [PATCH v2 2/3] hw/smbios: Use qapi for SmbiosEntryPointType

On Mon, 14 Dec 2020 15:50:28 -0500
Eduardo Habkost  wrote:

> This prepares for exposing the SMBIOS entry point type as a
> machine property on x86.
> 
> Based on a patch from Daniel P. Berrangé.
> 
> Signed-off-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: Igor Mammedov 

> ---
> First version of this code was submitted at:
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com
> 
> Changes from v1:
> * Patch was split in two
> * Declarations were moved to qapi/smbios.json
> * Documentation was updated to use the same terminology used in
>   SMBIOS documentation
> * Documentation was updated to "Since: 6.0"
> ---
>  qapi/qapi-schema.json|  1 +
>  qapi/smbios.json | 11 +++
>  include/hw/firmware/smbios.h | 10 ++
>  qapi/meson.build |  1 +
>  4 files changed, 15 insertions(+), 8 deletions(-)
>  create mode 100644 qapi/smbios.json
> 
> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 0b444b76d2..87a183fb13 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -86,6 +86,7 @@
>  { 'include': 'machine.json' }
>  { 'include': 'machine-target.json' }
>  { 'include': 'replay.json' }
> +{ 'include': 'smbios.json' }
>  { 'include': 'misc.json' }
>  { 'include': 'misc-target.json' }
>  { 'include': 'audio.json' }
> diff --git a/qapi/smbios.json b/qapi/smbios.json
> new file mode 100644
> index 00..55b3bd2e83
> --- /dev/null
> +++ b/qapi/smbios.json
> @@ -0,0 +1,11 @@
> +##
> +# @SmbiosEntryPointType:
> +#
> +# @2_1: SMBIOS version 2.1 (32-bit) Entry Point
> +#
> +# @3_0: SMBIOS version 3.0 (64-bit) Entry Point
> +#
> +# Since: 6.0
> +##
> +{ 'enum': 'SmbiosEntryPointType',
> +  'data': [ '2_1', '3_0' ] }
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 5467ecec78..b3beef1606 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -1,6 +1,8 @@
>  #ifndef QEMU_SMBIOS_H
>  #define QEMU_SMBIOS_H
>  
> +#include "qapi/qapi-types-smbios.h"
> +
>  /*
>   * SMBIOS Support
>   *
> @@ -23,14 +25,6 @@ struct smbios_phys_mem_area {
>  uint64_t length;
>  };
>  
> -/*
> - * SMBIOS spec defined tables
> - */
> -typedef enum SmbiosEntryPointType {
> -SMBIOS_ENTRY_POINT_TYPE_2_1,
> -SMBIOS_ENTRY_POINT_TYPE_3_0,
> -} SmbiosEntryPointType;
> -
>  /* SMBIOS Entry Point
>   * There are two types of entry points defined in the SMBIOS specification
>   * (see below). BIOS must place the entry point(s) at a 16-byte-aligned
> diff --git a/qapi/meson.build b/qapi/meson.build
> index 0e98146f1f..f7fb73d41b 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -42,6 +42,7 @@ qapi_all_modules = [
>'replay',
>'rocker',
>'run-state',
> +  'smbios',
>'sockets',
>'tpm',
>'trace',

Re: [PATCH v3 2/8] acpi: Add addr offset in build_crs

On Wed, 23 Dec 2020 17:08:30 +0800
Jiahui Cen  wrote:

> AML needs Address Translation offset to describe how a bridge translates
> addresses accross the bridge when using an address descriptor, and
> especially on ARM, the translation offset of pio resource is usually
> non zero.

could you point out where in patch [8/8] it becomes non zero?

> 
> Therefore, it's necessary to pass offset for pio, mmio32, mmio64 and bus
> number into build_crs.
> 
> Signed-off-by: Jiahui Cen 
> ---
>  hw/acpi/aml-build.c | 18 ++
>  hw/i386/acpi-build.c|  3 ++-
>  hw/pci-host/gpex-acpi.c |  3 ++-
>  include/hw/acpi/aml-build.h |  4 +++-
>  4 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index f976aa667b..7b6ebb0cc8 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2076,7 +2076,9 @@ void build_tpm2(GArray *table_data, BIOSLinker *linker, 
> GArray *tcpalog)
>   tpm2_ptr, "TPM2", table_data->len - tpm2_start, 4, NULL, 
> NULL);
>  }
>  
> -Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
> +Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set, uint32_t 
> io_offset,
> +   uint32_t mmio32_offset, uint64_t mmio64_offset,
> +   uint16_t bus_nr_offset)
>  {
>  Aml *crs = aml_resource_template();
>  CrsRangeSet temp_range_set;
> @@ -2189,10 +2191,10 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet 
> *range_set)
>  for (i = 0; i < temp_range_set.io_ranges->len; i++) {
>  entry = g_ptr_array_index(temp_range_set.io_ranges, i);
>  aml_append(crs,
> -   aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
> -   AML_POS_DECODE, AML_ENTIRE_RANGE,
> -   0, entry->base, entry->limit, 0,
> -   entry->limit - entry->base + 1));
> +   aml_dword_io(AML_MIN_FIXED, AML_MAX_FIXED,
> +AML_POS_DECODE, AML_ENTIRE_RANGE,
> +0, entry->base, entry->limit, io_offset,
> +entry->limit - entry->base + 1));
>  crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
>  }
>  
> @@ -2205,7 +2207,7 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet 
> *range_set)
> aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED,
>  AML_MAX_FIXED, AML_NON_CACHEABLE,
>  AML_READ_WRITE,
> -0, entry->base, entry->limit, 0,
> +0, entry->base, entry->limit, 
> mmio32_offset,
>  entry->limit - entry->base + 1));
>  crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
>  }
> @@ -2217,7 +2219,7 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet 
> *range_set)
> aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED,
>  AML_MAX_FIXED, AML_NON_CACHEABLE,
>  AML_READ_WRITE,
> -0, entry->base, entry->limit, 0,
> +0, entry->base, entry->limit, 
> mmio64_offset,
>  entry->limit - entry->base + 1));
>  crs_range_insert(range_set->mem_64bit_ranges,
>   entry->base, entry->limit);
> @@ -2230,7 +2232,7 @@ Aml *build_crs(PCIHostState *host, CrsRangeSet 
> *range_set)
>  0,
>  pci_bus_num(host->bus),
>  max_bus,
> -0,
> +bus_nr_offset,
>  max_bus - pci_bus_num(host->bus) + 1));
>  
>  return crs;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f18b71dea9..f56d699c7f 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1360,7 +1360,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>  }
>  
>  aml_append(dev, build_prt(false));
> -crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), 
> &crs_range_set);
> +crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), 
> &crs_range_set,
> +0, 0, 0, 0);
>  aml_append(dev, aml_name_decl("_CRS", crs));
>  aml_append(scope, dev);
>  aml_append(dsdt, scope);
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 7f20ee1c98..11b3db8f71 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -168,7 +168,8 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>   * 1. The resources the pci-brige/pcie-root-port need.
>   * 2. The resources the devices behind pxb need.
>   */
> -crs = build_crs(P

Re: [PATCH v3 3/8] acpi/gpex: Inform os to keep firmware resource map

On Wed, 23 Dec 2020 17:08:31 +0800
Jiahui Cen  wrote:

> There may be some differences in pci resource assignment between guest os
> and firmware.
> 
> Eg. A Bridge with Bus [d2]
> -+-[:d2]---01.0-[d3]01.0
> 
> where [d2:01.00] is a pcie-pci-bridge with BAR0 (mem, 64-bit, non-pref) 
> [size=256]
>   [d3:01.00] is a PCI Device with BAR0 (mem, 64-bit, pref) [size=128K]
>   BAR4 (mem, 64-bit, pref) [size=64M]
> 
> In EDK2, the Resource Map would be:
> PciBus: Resource Map for Bridge [D2|01|00]
> Type = PMem64; Base = 0x800400; Length = 0x410; 
> Alignment = 0x3FF
>Base = 0x800400; Length = 0x400; Alignment = 
> 0x3FF;  Owner = PCI [D3|01|00:20]
>Base = 0x800800; Length = 0x2;   Alignment = 0x1;  
>   Owner = PCI [D3|01|00:10]
> Type =  Mem64; Base = 0x800810; Length = 0x100; Alignment = 
> 0xFFF
> It would use 0x410 to calculate the root bus's PMem64 resource window.
> 
> While in Linux, kernel will use 0x1FF as the alignment to calculate
> the PMem64 size, which would be 0x600. So kernel would try to
> allocate 0x600 from the PMem64 resource window, but since the window
> size is 0x410 as assigned by EDK2, the allocation would fail.
> 
> The diffences could result in resource assignment failure.
> 
> Using _DSM #5 method to inform guest os not to ignore the PCI configuration
> that firmware has done at boot time could handle the differences.

I'm not sure about this one, 
OS should able to reconfigure PCI resources according to what and where is 
plugged
(and it even more true is hotplug is taken into account)

> 
> Signed-off-by: Jiahui Cen 
> ---
>  hw/pci-host/gpex-acpi.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 11b3db8f71..c189306599 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -112,10 +112,24 @@ static void acpi_dsdt_add_pci_osc(Aml *dev)
>  UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
>  ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>  ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> -uint8_t byte_list[1] = {1};
> -buf = aml_buffer(1, byte_list);
> +uint8_t byte_list[] = {
> +0x1 << 0 /* support for functions other than function 0 */ |
> +0x1 << 5 /* support for function 5 */
> +};
> +buf = aml_buffer(ARRAY_SIZE(byte_list), byte_list);
>  aml_append(ifctx1, aml_return(buf));
>  aml_append(ifctx, ifctx1);
> +
> +/* PCI Firmware Specification 3.1
> + * 4.6.5. _DSM for Ignoring PCI Boot Configurations
> + */
> +/* Arg2: Function Index: 5 */
> +ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(5)));
> +/* 0 - The operating system must not ignore the PCI configuration that
> + * firmware has done at boot time.
> + */
> +aml_append(ifctx1, aml_return(aml_int(0)));
> +aml_append(ifctx, ifctx1);
>  aml_append(method, ifctx);
>  
>  byte_list[0] = 0;

Re: [PATCH v3 5/8] acpi/gpex: Append pxb devs in ascending order

On Wed, 23 Dec 2020 17:08:33 +0800
Jiahui Cen  wrote:

> The overlap check of IO resource window would fail when Linux kernel
> registers an IO resource [b, c) earlier than another resource [a, b).
> Though this incorrect check could be fixed by [1], it would be better to
> append pxb devs into DSDT table in ascending order.
> 
> [1]: https://lore.kernel.org/lkml/20201218062335.5320-1-cenjia...@huawei.com/

considering there is acceptable fix for kernel I'd rather avoid
workarounds on QEMU side. So I suggest dropping this patch.

it also should reduce noise in [8/8] masking other changes.

> Signed-off-by: Jiahui Cen 
> ---
>  hw/pci-host/gpex-acpi.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 4bf1e94309..95a7a0f12b 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -141,7 +141,7 @@ static void acpi_dsdt_add_pci_osc(Aml *dev)
>  void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>  {
>  int nr_pcie_buses = cfg->ecam.size / PCIE_MMCFG_SIZE_MIN;
> -Aml *method, *crs, *dev, *rbuf;
> +Aml *method, *crs, *dev, *rbuf, *pxb_devs[nr_pcie_buses];
>  PCIBus *bus = cfg->bus;
>  CrsRangeSet crs_range_set;
>  CrsRangeEntry *entry;
> @@ -149,6 +149,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>  
>  /* start to construct the tables for pxb */
>  crs_range_set_init(&crs_range_set);
> +memset(pxb_devs, 0, sizeof(pxb_devs));
>  if (bus) {
>  QLIST_FOREACH(bus, &bus->child, sibling) {
>  uint8_t bus_num = pci_bus_num(bus);
> @@ -190,7 +191,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>  
>  acpi_dsdt_add_pci_osc(dev);
>  
> -aml_append(scope, dev);
> +pxb_devs[bus_num] = dev;
>  }
>  }
>  
> @@ -278,5 +279,11 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>  aml_append(dev, dev_res0);
>  aml_append(scope, dev);
>  
> +for (i = 0; i < ARRAY_SIZE(pxb_devs); i++) {
> +if (pxb_devs[i]) {
> +aml_append(scope, pxb_devs[i]);
> +}
> +}
> +
>  crs_range_set_free(&crs_range_set);
>  }

Re: [PATCH v3 6/8] Kconfig: Enable PXB for ARM_VIRT by default

On Wed, 23 Dec 2020 17:08:34 +0800
Jiahui Cen  wrote:
subj
s/by default//
s/enable/compile/

> PXB is now supported on ARM, so let's enable it by default.
s/it by default/for arm_virt machine/
s/enable/compile/

> 
> Signed-off-by: Jiahui Cen 
> ---
>  hw/pci-bridge/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci-bridge/Kconfig b/hw/pci-bridge/Kconfig
> index a51ec716f5..f8df4315ba 100644
> --- a/hw/pci-bridge/Kconfig
> +++ b/hw/pci-bridge/Kconfig
> @@ -5,7 +5,7 @@ config PCIE_PORT
>  
>  config PXB
>  bool
> -default y if Q35
> +default y if Q35 || ARM_VIRT
>  
>  config XIO3130
>  bool

Re: [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow


On 28/12/2020 07:07, David Gibson wrote:


On Sat, Dec 19, 2020 at 10:42:24AM +, Mark Cave-Ayland wrote:

Signed-off-by: Mark Cave-Ayland 


Looks correct, but it could really do with a rationale in the commit message.


It's just changing the order of initialisation since as the plan is to move the PIC 
to the macio device, the PCI bus needs to exist before the macio device is initiated 
and also before wiring up to the PIC. I'll come up with something along those lines 
for a v2.



---
  hw/ppc/mac_oldworld.c | 30 +++---
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2ead34bdf1..e58e0525fe 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -227,6 +227,21 @@ static void ppc_heathrow_init(MachineState *machine)
  }
  }
  
+/* Grackle PCI host bridge */

+dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, &error_fatal);
+
+sysbus_mmio_map(s, 0, GRACKLE_BASE);
+sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
+/* PCI hole */
+memory_region_add_subregion(get_system_memory(), 0x8000ULL,
+sysbus_mmio_get_region(s, 2));
+/* Register 2 MB of ISA IO space */
+memory_region_add_subregion(get_system_memory(), 0xfe00,
+sysbus_mmio_get_region(s, 3));
+
  /* XXX: we register only 1 output pin for heathrow PIC */
  pic_dev = qdev_new(TYPE_HEATHROW);
  sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
@@ -251,21 +266,6 @@ static void ppc_heathrow_init(MachineState *machine)
  tbfreq = TBFREQ;
  }
  
-/* Grackle PCI host bridge */

-dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-s = SYS_BUS_DEVICE(dev);
-sysbus_realize_and_unref(s, &error_fatal);
-
-sysbus_mmio_map(s, 0, GRACKLE_BASE);
-sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
-/* PCI hole */
-memory_region_add_subregion(get_system_memory(), 0x8000ULL,
-sysbus_mmio_get_region(s, 2));
-/* Register 2 MB of ISA IO space */
-memory_region_add_subregion(get_system_memory(), 0xfe00,
-sysbus_mmio_get_region(s, 3));
-
  for (i = 0; i < 4; i++) {
  qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
  }



ATB,

Mark.

Re: Bug in Bonito? (mips/fuloong2e)

On 12/29/20 2:02 PM, Jiaxun Yang wrote:
> 在 2020/12/29 18:47, Philippe Mathieu-Daudé 写道:
>> On 12/29/20 6:26 AM, Jiaxun Yang wrote:
>>> 在 2020/12/29 上午11:26, BALATON Zoltan 写道:
 Hello,

 While continuing with part two of my vt82c686b clean ups I've tried to
 implement SMBus IO base configuration in the vt82c686b-pm part that
 I've already done for vt8231 for pegasos2 and it should be the same
 for 686B. (In short, writing address to pm config 0x90 sets base
 address of smbus regs and bit 0 of 0xd2 enables/disables it.) This is
 what the firmware does first and it would allow removing hard coded
 0xeee1 value and the property to set it and then I could reuse the
 same PM part in VT8231.

>>> [...]
 Any idea what this is trying to do and how to fix it?
>>> It's trying to translate Bonito style PCI config space r/w to
>>> standard PCI
>>> config space R/W.
>>>
>>> A quick galance told me change BONITO_PCICONF_REG_MASK to 0xff
>>> may help.
>> Per the datasheet section "5.7.5. Accessing PCI configuration space"
>> 0xfc is the correct value, but the register number starts at the 2nd
>> bit. So this is not a write access to register 0xd2 but 0x34?
> 
> It seems like Loongson changed defination of the register?

Maybe, I only have the bonito64 specs, not the Loongson2 ones.
I am a bit confused, I thought the Fuloong 2E was based on
bonito64 (which QEMU models).

Do you know if the Loongson2 specs are public?

> There is no shifting in kernel[1] as well.
> 
> ```
> /* Type 1 configuration for offboard PCI bus */
> addr = (busnum << 16) | (device << 11) | (function << 8) | reg;
> ```

OK, this makes sense after looking at Linux kernel commit e2fee5723bbd
("MIPS: Bonito64: Make Loongson independent from Bonito64 code.") [2]

I'm a bit reluctant to modify hw/pci-host/bonito.c to make Loongson2
works without having the specs handy, justifying simply because
"Linux uses it that way".

OTOH it is pointless to maintain a model that has no users (thinking
about not breaking the bonito64 model).

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e2fee5723bbd

> 
> Thanks.
> 
> [1]:
> https://elixir.bootlin.com/linux/latest/source/arch/mips/pci/ops-loongson2.c
> 
> 
> - Jiaxun
>>
>> If you can test, this is the snippet I plan to send later:
>>
>> -- >8 --
>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>> index a99eced0657..65953766dd0 100644
>> --- a/hw/pci-host/bonito.c
>> +++ b/hw/pci-host/bonito.c
>> @@ -189,3 +189,3 @@ FIELD(BONGENCFG, PCIQUEUE,  12, 1)
>>   #define BONITO_PCICONF_REG_MASK    0xFC
>> -#define BONITO_PCICONF_REG_OFFSET  0
>> +#define BONITO_PCICONF_REG_OFFSET  2
>> ---
>>
>>> Thanks.
>>>
>>> - Jiaxun
>>>
 Regards,
 BALATON Zoltan
>>>
> 
>

Re: [PATCH v2 2/2] via-ide: Fix fuloong2e support


On Tue, 29 Dec 2020, Mark Cave-Ayland wrote:

On 29/12/2020 12:01, BALATON Zoltan via wrote:
Fortunately with PCI_CLASS_PROG at 0x8a Linux will keep the VIA IDE in 
compatible mode and not attempt to switch to native mode: therefore if you 
keep this as-is and add the legacy IDE ioports back, that just leaves the 
problem with BAR4 (BMDMA). In effect your patch isn't setting compatible 
mode anymore, it is just disabling BMDMA.


It's actually Guenter's patch so you're now bashing him not me...


This is a deliberately misleading comment, and not a good introduction for


It's not deliberately misleading just stating a fact that you deliberately 
misinterpret. Guenter has contributed before (he also fixed up my sam460ex 
emulation) and his contributions are certainly appreciated. I'm sorry that 
he got unintentinally invoved in this disagreement between us two but I 
think he can ignore all this without a problem. I did not mean to drag him 
into it, just pointed out what you're doing and I think he got that.


someone external becoming involved with the project. Guenter's patch was a 
PoC demonstrating how to fix the fuloong2e machine, which is really 
appreciated since it clearly locates the problems to allow a fix to be 
applied upstream.


(But I also think your time could be better spent than getting rid of this 
hack when there are much more hacks or missing functionalities to get rid 
of in the part you maintain.)


And comments like this are not appropriate for a technical mailing list 
either. I've done my best to review your patch in good faith (including


Don't misunderstand this again, I did not mean to say that you made 
mistakes (although everyone does so that's not a problem) but what I meant 
is that there are a lot of things even in your areas that could be 
improved and that's time much better spent than discussing this patch 
endlessly on phylosophical basis when it's unlikely to get better.


reading related specifications and testing your pegasos2 model) and explain 
why it isn't reporting the correct information to the guest.


Yes I agree this should be brought to off list and clear up this between 
us I just don't have time for it now but I'll write to you later. As for 
your comments, we've been through all this in March and I get the 
impression that whatever I submit is criticised by you all the time so I 
really wonder if it's against me personally or you just getting old and 
grumpy :-) Don't take this as personal, no insult is meant just want to 
know if I did something that made you change your mind about my 
contributions or you do this to everything submitted to QEMU, because 
that's slowly becomes a burden discouriging me to contribute anything. I 
already gave up contributing to OpenBIOS but won't give up with QEMU, but 
I'm a bit tired having to fight for every little change to get past you 
(even in areas you're not maintaining like this one).


You can answer in private to this, I think others will be greateful to be 
left out of the discussion.


Phil - I hope you find that found my review comments useful and that they 
explain why the patchset is wrong by always claiming legacy IDE ioports exist 
but not providing them unless the new option is set (and indeed indicating 
some of the shortcomings of QEMU related to PCI BARs in this area that can be 
improved in future). As I feel comments in this thread have become directed 
at me personally, I am going to take a step back from this.


I'm sorry if you feel my replies got personal but I also feel your 
comments getting at me and you seemed to critique something that was 
addmittedly not perfect but working and demanded perfection where it's not 
feasible (without way more work that you can expect from unpayed 
contributors) and calling my patches wrong for that reason. I don't mind 
if you add comments, warnnings or change the commit message to say "this 
patch is all wrong but fixes Linux on fuloong2e and makes guests work with 
pegasos2 within the constraints of QEMU" as long as it gets in until a 
better fix may be available sometimes in the future. But:


- this patch is not mine now
- I did change what you asked in the first review but then you came back with 
this
- all this has been discussed to death and everyone but you seemed to accept it

I'll try to refrain from answering any more about this and let's continue 
off list if you want. I'd really want to avoid further confrontation but I 
happen to contribute to parts you also have an interest in so it's hard to 
avid each other so it would be better to get to some acceptable terms that 
allow me to contribute without upsetting you too much.


Regards,
BALATON Zoltan

Re: [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device


On 28/12/2020 07:08, David Gibson wrote:


On Sat, Dec 19, 2020 at 10:42:25AM +, Mark Cave-Ayland wrote:

Really needs a commit message.


This is currently explained in the cover letter: it's moving the PIC to the macio 
device as per real hardware (which also nicely removes the need for compulsory object 
property links which currently trip up some of the automated QOM introspection 
tests). I'll add something along these lines for the next revision.



Signed-off-by: Mark Cave-Ayland 
---
  hw/misc/macio/macio.c | 20 +--
  hw/ppc/mac_oldworld.c | 66 +--
  include/hw/misc/macio/macio.h |  2 +-
  3 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index bb601f782c..cfb87da6c9 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -140,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
  {
  MacIOState *s = MACIO(d);
  OldWorldMacIOState *os = OLDWORLD_MACIO(d);
-DeviceState *pic_dev = DEVICE(os->pic);
+DeviceState *pic_dev = DEVICE(&os->pic);
  Error *err = NULL;
  SysBusDevice *sysbus_dev;
  
@@ -150,6 +150,14 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)

  return;
  }
  
+/* Heathrow PIC */

+if (!qdev_realize(DEVICE(&os->pic), BUS(&s->macio_bus), errp)) {
+return;
+}
+sysbus_dev = SYS_BUS_DEVICE(&os->pic);
+memory_region_add_subregion(&s->bar, 0x0,
+sysbus_mmio_get_region(sysbus_dev, 0));
+
  qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
   s->frequency);
  if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
@@ -175,11 +183,6 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
  sysbus_mmio_get_region(sysbus_dev, 0));
  pmac_format_nvram_partition(&os->nvram, os->nvram.size);
  
-/* Heathrow PIC */

-sysbus_dev = SYS_BUS_DEVICE(os->pic);
-memory_region_add_subregion(&s->bar, 0x0,
-sysbus_mmio_get_region(sysbus_dev, 0));
-
  /* IDE buses */
  macio_realize_ide(s, &os->ide[0],
qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
@@ -218,10 +221,7 @@ static void macio_oldworld_init(Object *obj)
  DeviceState *dev;
  int i;
  
-object_property_add_link(obj, "pic", TYPE_HEATHROW,

- (Object **) &os->pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+object_initialize_child(OBJECT(s), "pic", &os->pic, TYPE_HEATHROW);
  
  object_initialize_child(OBJECT(s), "cuda", &s->cuda, TYPE_CUDA);
  
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c

index e58e0525fe..44ee99be88 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
  MACIOIDEState *macio_ide;
  ESCCState *escc;
  SysBusDevice *s;
-DeviceState *dev, *pic_dev;
+DeviceState *dev, *pic_dev, *grackle_dev;
  BusState *adb_bus;
  uint64_t bios_addr;
  int bios_size;
@@ -227,10 +227,17 @@ static void ppc_heathrow_init(MachineState *machine)
  }
  }
  
+/* Timebase Frequency */

+if (kvm_enabled()) {
+tbfreq = kvmppc_get_tbfreq();
+} else {
+tbfreq = TBFREQ;
+}
+
  /* Grackle PCI host bridge */
-dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-s = SYS_BUS_DEVICE(dev);
+grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x8000);
+s = SYS_BUS_DEVICE(grackle_dev);
  sysbus_realize_and_unref(s, &error_fatal);
  
  sysbus_mmio_map(s, 0, GRACKLE_BASE);

@@ -242,14 +249,30 @@ static void ppc_heathrow_init(MachineState *machine)
  memory_region_add_subregion(get_system_memory(), 0xfe00,
  sysbus_mmio_get_region(s, 3));
  
-/* XXX: we register only 1 output pin for heathrow PIC */

-pic_dev = qdev_new(TYPE_HEATHROW);
-sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
+pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
+
+/* MacIO */
+macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
+dev = DEVICE(macio);
+qdev_prop_set_uint64(dev, "frequency", tbfreq);
+
+escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
+pci_realize_and_unref(macio, pci_bus, &error_fatal);
+
+pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(grackle_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x15 + i));
+}

Re: [PATCH v2 0/3] pc: Support configuration of SMBIOS entry point type

On 12/14/20 9:50 PM, Eduardo Habkost wrote:
> This includes code previously submitted[1] by Daniel P. Berrangé
> to add a "smbios-ep" machine property on PC.
> 
> SMBIOS 3.0 is necessary to support more than ~720 VCPUs, as a
> large number of VCPUs can easily hit the table size limit of
> SMBIOS 2.1 entry points.
> 
> [1] 
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-5-berra...@redhat.com
> 
> https://lore.kernel.org/qemu-devel/20200908165438.1008942-6-berra...@redhat.com
> 
> Daniel P. Berrangé (1):
>   hw/i386: expose a "smbios-ep" PC machine property
> 
> Eduardo Habkost (2):
>   smbios: Rename SMBIOS_ENTRY_POINT_* enums
>   hw/smbios: Use qapi for SmbiosEntryPointType
> 
>  qapi/qapi-schema.json|  1 +
>  qapi/smbios.json | 11 +++
>  include/hw/firmware/smbios.h | 10 ++
>  include/hw/i386/pc.h |  3 +++
>  hw/arm/virt.c|  2 +-
>  hw/i386/pc.c | 26 ++
>  hw/i386/pc_piix.c|  2 +-
>  hw/i386/pc_q35.c |  2 +-
>  hw/smbios/smbios.c   |  8 
>  qapi/meson.build |  1 +
>  10 files changed, 51 insertions(+), 15 deletions(-)
>  create mode 100644 qapi/smbios.json

Reviewed-by: Philippe Mathieu-Daudé

[Bug 1908266] Re: spice unnecessary forces nographic

2020-12-29 Thread Tom Yan

The gtk window is not limited for -display but also for compatmonitor /
serial /paralel, but when -spice is used, the gtk window does not show
at all. While you can force the window to show with -display gtk, but
the *side effect* is the vga will be wired/connected to the gtk window
(which seems to break things when gl and so on is enabled).

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1908266

Title:
  spice unnecessary forces nographic

Status in QEMU:
  Incomplete

Bug description:
  When spice is enabled, qemu does not give the graphical window. It
  should not imply -nographic but only -display none.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1908266/+subscriptions

Re: [PATCH] multi-process: Acceptance test for multiprocess QEMU

2020-12-29 Thread Jag Raman




> On Dec 23, 2020, at 1:49 PM, Elena Ufimtseva  
> wrote:
> 
> On Wed, Dec 23, 2020 at 03:01:24PM +0400, Marc-André Lureau wrote:
>> Hi
>> 
>> On Wed, Dec 23, 2020 at 10:45 AM  wrote:
>> 
>>> From: Jagannathan Raman 
>>> 
>>> Runs the Avocado acceptance test to check if a
>>> remote lsi53c895a device gets identified by the guest.
>>> 
>>> Signed-off-by: Elena Ufimtseva 
>>> Signed-off-by: John G Johnson 
>>> Signed-off-by: Jagannathan Raman 
>>> ---
>>> tests/acceptance/multiprocess.py | 104 +++
>>> 1 file changed, 104 insertions(+)
>>> create mode 100644 tests/acceptance/multiprocess.py
>>> 
>>> diff --git a/tests/acceptance/multiprocess.py
>>> b/tests/acceptance/multiprocess.py
>>> new file mode 100644
>>> index 00..d10b4d2c05
>>> --- /dev/null
>>> +++ b/tests/acceptance/multiprocess.py
>>> @@ -0,0 +1,104 @@
>>> +# Test for multiprocess qemu
>>> +#
>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>> +# later.  See the COPYING file in the top-level directory.
>>> +
>>> +
>>> +from avocado_qemu import Test
>>> +from avocado_qemu import wait_for_console_pattern
>>> +from avocado_qemu import exec_command_and_wait_for_pattern
>>> +
>>> +from qemu.accel import kvm_available
>>> +
>>> +import os
>>> +import socket
>>> +
>>> +ACCEL_NOT_AVAILABLE_FMT = "%s accelerator does not seem to be available"
>>> +KVM_NOT_AVAILABLE = ACCEL_NOT_AVAILABLE_FMT % "KVM"
>>> +
>>> +class Multiprocess(Test):
>>> +"""
>>> +:avocado: tags=multiprocess
>>> +"""
>>> +KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
>>> +
>>> +def wait_for_console_pattern(self, success_message, vm=None):
>>> +wait_for_console_pattern(self, success_message,
>>> + failure_message='Kernel panic - not
>>> syncing',
>>> + vm=vm)
>>> +
>>> +def do_test(self, kernel_url, initrd_url, kernel_command_line,
>>> +machine_type):
>>> +if not kvm_available(self.arch, self.qemu_bin):
>>> +self.cancel(KVM_NOT_AVAILABLE)
>>> +
>>> +# Create socketpair to connect proxy and remote processes
>>> +proxy_sock, remote_sock = socket.socketpair(socket.AF_UNIX,
>>> +socket.SOCK_STREAM)
>>> +os.set_inheritable(proxy_sock.fileno(), True)
>>> +os.set_inheritable(remote_sock.fileno(), True)
>>> +
>>> +kernel_path = self.fetch_asset(kernel_url)
>>> +initrd_path = self.fetch_asset(initrd_url)
>>> +
>>> +# Create remote process
>>> +remote_vm = self.get_vm()
>>> +remote_vm.add_args('-machine', 'x-remote')
>>> +remote_vm.add_args('-nodefaults')
>>> +remote_vm.add_args('-device', 'lsi53c895a,id=lsi1')
>>> +remote_vm.add_args('-object', 'x-remote-object,id=robj1,'
>>> +   'devid=lsi1,fd='+str(remote_sock.fileno()))
>>> +remote_vm.launch()
>>> +
>>> +# Create proxy process
>>> +self.vm.set_console()
>>> +self.vm.add_args('-machine', machine_type)
>>> +self.vm.add_args('-accel', 'kvm')
>>> +self.vm.add_args('-cpu', 'host')
>>> +self.vm.add_args("-object",
>>> + "memory-backend-memfd,id=sysmem-file,size=2G")
>>> +self.vm.add_args("--numa", "node,memdev=sysmem-file")
>>> +self.vm.add_args("-m", "2048")
>>> +self.vm.add_args('-kernel', kernel_path,
>>> + '-initrd', initrd_path,
>>> + '-append', kernel_command_line)
>>> +self.vm.add_args('-device',
>>> + 'x-pci-proxy-dev,'
>>> + 'id=lsi1,fd='+str(proxy_sock.fileno()))
>>> +self.vm.launch()
>>> +self.wait_for_console_pattern("as init process")
>>> +exec_command_and_wait_for_pattern(self, "mount -t sysfs sysfs
>>> /sys",
>>> +  '', '')
>>> +exec_command_and_wait_for_pattern(self,
>>> +  "cat
>>> /sys/bus/pci/devices/*/uevent",
>>> +  "PCI_ID=1000:0012", '')
>>> +
>>> +def test_multiprocess_x86_64(self):
>>> +"""
>>> +:avocado: tags=arch:x86_64
>>> +"""
>>> +kernel_url = ('
>>> https://archives.fedoraproject.org/pub/archive/fedora'
>>> +  '/linux/releases/31/Everything/x86_64/os/images'
>>> +  '/pxeboot/vmlinuz')
>>> +initrd_url = ('
>>> https://archives.fedoraproject.org/pub/archive/fedora'
>>> +  '/linux/releases/31/Everything/x86_64/os/images'
>>> +  '/pxeboot/initrd.img')
>>> +kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>>> +   'console=ttyS0 rdinit=/bin/bash')
>>> +machine = 'pc'
>>> +self.do_test(kernel_url, initrd_url, kernel_command_line, mac

Re: [PATCH] gdb: riscv: Add target description

2020-12-29 Thread Sylvain Pelissier

Thank you for your remark here is the new patch:

Signed-off-by: Sylvain Pelissier 
---
 target/riscv/cpu.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 254cd83f8b..ed4971978b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -556,6 +556,18 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };

+static gchar *riscv_gdb_arch_name(CPUState *cs)
+{
+RISCVCPU *cpu = RISCV_CPU(cs);
+CPURISCVState *env = &cpu->env;
+
+if (riscv_cpu_is_32bit(env)) {
+return g_strdup("riscv:rv32");
+} else {
+return g_strdup("riscv:rv64");
+}
+}
+
 static void riscv_cpu_class_init(ObjectClass *c, void *data)
 {
 RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
@@ -591,6 +603,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
*data)
 /* For now, mark unmigratable: */
 cc->vmsd = &vmstate_riscv_cpu;
 #endif
+cc->gdb_arch_name = riscv_gdb_arch_name;
 #ifdef CONFIG_TCG
 cc->tcg_initialize = riscv_translate_init;
 cc->tlb_fill = riscv_cpu_tlb_fill;
-- 
2.25.1

Regards

Sylvain

On Tue, 29 Dec 2020 at 05:11, Bin Meng  wrote:

> On Thu, Dec 24, 2020 at 1:09 AM Sylvain Pelissier
>  wrote:
> >
> > Target description is not currently implemented in RISC-V architecture.
> Thus GDB won't set it properly when attached. The patch implements the
> target description response.
> >
> > Signed-off-by: Sylvain Pelissier 
> > ---
> >  target/riscv/cpu.c | 10 ++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 254cd83f8b..489d66038c 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -556,6 +556,15 @@ static Property riscv_cpu_properties[] = {
> >  DEFINE_PROP_END_OF_LIST(),
> >  };
> >
> > +static gchar *riscv_gdb_arch_name(CPUState *cs)
> > +{
> > +#ifdef TARGET_RISCV64
>
> Use riscv_cpu_is_32bit() instead of #ifdefs
>
> > +return g_strdup("riscv:rv64");
> > +#else
> > +return g_strdup("riscv:rv32");
> > +#endif
> > +}
> > +
> >  static void riscv_cpu_class_init(ObjectClass *c, void *data)
> >  {
> >  RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> > @@ -591,6 +600,7 @@ static void riscv_cpu_class_init(ObjectClass *c,
> void *data)
> >  /* For now, mark unmigratable: */
> >  cc->vmsd = &vmstate_riscv_cpu;
> >  #endif
> > +cc->gdb_arch_name = riscv_gdb_arch_name;
> >  #ifdef CONFIG_TCG
> >  cc->tcg_initialize = riscv_translate_init;
> >  cc->tlb_fill = riscv_cpu_tlb_fill;
> > --
>
> Regards,
> Bin
>

[PATCH] Deprecate pmem=on with non-DAX capable backend file

It is not safe to pretend that emulated NVDIMM supports
persistence while backend actually failed to enable it
and used non-persistent mapping as fall back.
Instead of falling-back, QEMU should be more strict and
error out with clear message that it's not supported.
So if user asks for persistence (pmem=on), they should
store backing file on NVDIMM.

Signed-off-by: Igor Mammedov 
---
 docs/system/deprecated.rst | 14 ++
 util/mmap-alloc.c  |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index bacd76d7a5..ba4f6ed2fe 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -327,6 +327,20 @@ The Raspberry Pi machines come in various models (A, A+, 
B, B+). To be able
 to distinguish which model QEMU is implementing, the ``raspi2`` and ``raspi3``
 machines have been renamed ``raspi2b`` and ``raspi3b``.
 
+Backend options
+---
+
+Using non-persistent backing file with pmem=on (since 6.0)
+''
+
+This option is used when ``memory-backend-file`` is consumed by emulated NVDIMM
+device. However enabling ``memory-backend-file.pmem`` option, when backing file
+is not DAX capable or not on a filesystem that support direct mapping of 
persistent
+memory, is not safe and may lead to data loss or corruption in case of host 
crash.
+Using pmem=on option with such file will return error, instead of a warning.
+Options are to move backing file to NVDIMM storage or modify VM configuration
+to set ``pmem=off`` to continue using fake NVDIMM without persistence 
guaranties.
+
 Device options
 --
 
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 27dcccd8ec..d226273a98 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/host-utils.h"
+#include "qemu/error-report.h"
 
 #define HUGETLBFS_MAGIC   0x958458f6
 
@@ -166,6 +167,8 @@ void *qemu_ram_mmap(int fd,
 "crash.\n", file_name);
 g_free(proc_link);
 g_free(file_name);
+warn_report("Deprecated using non DAX backing file with"
+" pmem=on option");
 }
 /*
  * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC,
-- 
2.27.0

[PATCH v2 2/7] mac_oldworld: move initialisation of grackle before heathrow

In order to move the heathrow PIC to the macio device, the PCI bus needs to be
initialised before the macio device and also before wiring the PIC IRQs.

Signed-off-by: Mark Cave-Ayland 
---
 hw/ppc/mac_oldworld.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2ead34bdf1..e58e0525fe 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -227,6 +227,21 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 }
 
+/* Grackle PCI host bridge */
+dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, &error_fatal);
+
+sysbus_mmio_map(s, 0, GRACKLE_BASE);
+sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
+/* PCI hole */
+memory_region_add_subregion(get_system_memory(), 0x8000ULL,
+sysbus_mmio_get_region(s, 2));
+/* Register 2 MB of ISA IO space */
+memory_region_add_subregion(get_system_memory(), 0xfe00,
+sysbus_mmio_get_region(s, 3));
+
 /* XXX: we register only 1 output pin for heathrow PIC */
 pic_dev = qdev_new(TYPE_HEATHROW);
 sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
@@ -251,21 +266,6 @@ static void ppc_heathrow_init(MachineState *machine)
 tbfreq = TBFREQ;
 }
 
-/* Grackle PCI host bridge */
-dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-s = SYS_BUS_DEVICE(dev);
-sysbus_realize_and_unref(s, &error_fatal);
-
-sysbus_mmio_map(s, 0, GRACKLE_BASE);
-sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
-/* PCI hole */
-memory_region_add_subregion(get_system_memory(), 0x8000ULL,
-sysbus_mmio_get_region(s, 2));
-/* Register 2 MB of ISA IO space */
-memory_region_add_subregion(get_system_memory(), 0xfe00,
-sysbus_mmio_get_region(s, 3));
-
 for (i = 0; i < 4; i++) {
 qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
 }
-- 
2.20.1

[PATCH v2 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs

This both allows the wiring to be done as Ben suggested in his original comment 
in
gpio.c and also enables the OpenPIC object property link to be removed.

Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/gpio.c | 24 +---
 hw/misc/macio/macio.c| 12 +++-
 include/hw/misc/macio/gpio.h |  2 --
 3 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 0fef8fb335..b1bcf830c3 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -57,10 +57,7 @@ void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool 
state)
 
 s->gpio_regs[gpio] = new_reg;
 
-/* This is will work until we fix the binding between MacIO and
- * the MPIC properly so we can route all GPIOs and avoid going
- * via the top level platform code.
- *
+/*
  * Note that we probably need to get access to the MPIC config to
  * decode polarity since qemu always use "raise" regardless.
  *
@@ -152,25 +149,15 @@ static const MemoryRegionOps macio_gpio_ops = {
 },
 };
 
-static void macio_gpio_realize(DeviceState *dev, Error **errp)
-{
-MacIOGPIOState *s = MACIO_GPIO(dev);
-
-s->gpio_extirqs[1] = qdev_get_gpio_in(DEVICE(s->pic),
-  NEWWORLD_EXTING_GPIO1);
-s->gpio_extirqs[9] = qdev_get_gpio_in(DEVICE(s->pic),
-  NEWWORLD_EXTING_GPIO9);
-}
-
 static void macio_gpio_init(Object *obj)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 MacIOGPIOState *s = MACIO_GPIO(obj);
+int i;
 
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) &s->pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+for (i = 0; i < 10; i++) {
+sysbus_init_irq(sbd, &s->gpio_extirqs[i]);
+}
 
 memory_region_init_io(&s->gpiomem, OBJECT(s), &macio_gpio_ops, obj,
   "gpio", 0x30);
@@ -207,7 +194,6 @@ static void macio_gpio_class_init(ObjectClass *oc, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(oc);
 NMIClass *nc = NMI_CLASS(oc);
 
-dc->realize = macio_gpio_realize;
 dc->reset = macio_gpio_reset;
 dc->vmsd = &vmstate_macio_gpio;
 nc->nmi_monitor_handler = macio_gpio_nmi;
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 36be77cede..d17cffd868 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -324,14 +324,16 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 
 if (ns->has_pmu) {
 /* GPIOs */
-sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
-object_property_set_link(OBJECT(&ns->gpio), "pic", OBJECT(pic_dev),
- &error_abort);
-memory_region_add_subregion(&s->bar, 0x50,
-sysbus_mmio_get_region(sysbus_dev, 0));
 if (!qdev_realize(DEVICE(&ns->gpio), BUS(&s->macio_bus), errp)) {
 return;
 }
+sysbus_dev = SYS_BUS_DEVICE(&ns->gpio);
+sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
+   NEWWORLD_EXTING_GPIO1));
+sysbus_connect_irq(sysbus_dev, 9, qdev_get_gpio_in(pic_dev,
+   NEWWORLD_EXTING_GPIO9));
+memory_region_add_subregion(&s->bar, 0x50,
+sysbus_mmio_get_region(sysbus_dev, 0));
 
 /* PMU */
 object_initialize_child(OBJECT(s), "pmu", &s->pmu, TYPE_VIA_PMU);
diff --git a/include/hw/misc/macio/gpio.h b/include/hw/misc/macio/gpio.h
index 4dee09a9dd..7d2aa886c2 100644
--- a/include/hw/misc/macio/gpio.h
+++ b/include/hw/misc/macio/gpio.h
@@ -38,8 +38,6 @@ struct MacIOGPIOState {
 SysBusDevice parent;
 /*< public >*/
 
-OpenPICState *pic;
-
 MemoryRegion gpiomem;
 qemu_irq gpio_extirqs[10];
 uint8_t gpio_levels[8];
-- 
2.20.1

[PATCH v2 0/7] macio: remove PIC object property links

This patchset follows on from the dicussion at 
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02630.html
where the user_creatable flag for the macio devices was set back to false just
before the 5.2 release.

The underlying cause was that the PIC object property links were not being set
before realise. Whilst this cannot happen when launching the g3beige and mac99
machines from qemu-system-ppc, it caused some automated tests to fail.

Here we fix the real problem which is to move the PIC for both machines into the
macio device, which not only matches real hardware but also enables the PIC 
object
property links to be completely removed.

Patch 6 rewires the macio gpios for the mac99 machine as per Ben's original 
comment
after the OpenPIC device has been moved into the macio-newworld device, and then
finally patch 7 removes setting the user_creatable flag to false on the macio 
devices
once again.

Signed-off-by: Mark Cave-Ayland 

v2:
- Add R-B tag for patch 1 from David
- Update commit messages to included more detail as requested by David


Mark Cave-Ayland (7):
  mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
  mac_oldworld: move initialisation of grackle before heathrow
  macio: move heathrow PIC inside macio-oldworld device
  mac_newworld: delay wiring of PCI IRQs in New World machine
  macio: move OpenPIC inside macio-newworld device
  macio: wire macio GPIOs to OpenPIC using sysbus IRQs
  macio: don't set user_creatable to false

 hw/misc/macio/gpio.c  | 24 +++
 hw/misc/macio/macio.c | 53 
 hw/ppc/mac_newworld.c | 71 
 hw/ppc/mac_oldworld.c | 76 ---
 include/hw/misc/macio/gpio.h  |  2 -
 include/hw/misc/macio/macio.h |  4 +-
 6 files changed, 104 insertions(+), 126 deletions(-)

-- 
2.20.1

[PATCH v2 5/7] macio: move OpenPIC inside macio-newworld device

The OpenPIC device is located within the macio device on real hardware so make 
it
a child of the macio-newworld device. This also removes the need for setting and
checking a separate PIC object property link on the macio-newworld device which
currently causes the automated QOM introspection tests to fail.

Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/macio.c | 19 +--
 hw/ppc/mac_newworld.c | 25 +++--
 include/hw/misc/macio/macio.h |  2 +-
 3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index cfb87da6c9..36be77cede 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -273,7 +273,7 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 {
 MacIOState *s = MACIO(d);
 NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
-DeviceState *pic_dev = DEVICE(ns->pic);
+DeviceState *pic_dev = DEVICE(&ns->pic);
 Error *err = NULL;
 SysBusDevice *sysbus_dev;
 MemoryRegion *timer_memory = NULL;
@@ -284,17 +284,19 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 return;
 }
 
+/* OpenPIC */
+qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
+sysbus_dev = SYS_BUS_DEVICE(&ns->pic);
+sysbus_realize_and_unref(sysbus_dev, &error_fatal);
+memory_region_add_subregion(&s->bar, 0x4,
+sysbus_mmio_get_region(sysbus_dev, 0));
+
 sysbus_dev = SYS_BUS_DEVICE(&s->escc);
 sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
NEWWORLD_ESCCB_IRQ));
 sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
NEWWORLD_ESCCA_IRQ));
 
-/* OpenPIC */
-sysbus_dev = SYS_BUS_DEVICE(ns->pic);
-memory_region_add_subregion(&s->bar, 0x4,
-sysbus_mmio_get_region(sysbus_dev, 0));
-
 /* IDE buses */
 macio_realize_ide(s, &ns->ide[0],
   qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
@@ -369,10 +371,7 @@ static void macio_newworld_init(Object *obj)
 NewWorldMacIOState *ns = NEWWORLD_MACIO(obj);
 int i;
 
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) &ns->pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+object_initialize_child(OBJECT(s), "pic", &ns->pic, TYPE_OPENPIC);
 
 object_initialize_child(OBJECT(s), "gpio", &ns->gpio, TYPE_MACIO_GPIO);
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 708bb2f1ab..e991db4add 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -293,18 +293,6 @@ static void ppc_core99_init(MachineState *machine)
 }
 }
 
-pic_dev = qdev_new(TYPE_OPENPIC);
-qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
-s = SYS_BUS_DEVICE(pic_dev);
-sysbus_realize_and_unref(s, &error_fatal);
-k = 0;
-for (i = 0; i < smp_cpus; i++) {
-for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
-sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
-}
-}
-g_free(openpic_irqs);
-
 if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
 /* 970 gets a U3 bus */
 /* Uninorth AGP bus */
@@ -378,8 +366,6 @@ static void ppc_core99_init(MachineState *machine)
 qdev_prop_set_uint64(dev, "frequency", tbfreq);
 qdev_prop_set_bit(dev, "has-pmu", has_pmu);
 qdev_prop_set_bit(dev, "has-adb", has_adb);
-object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
- &error_abort);
 
 escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
 qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
@@ -387,6 +373,7 @@ static void ppc_core99_init(MachineState *machine)
 
 pci_realize_and_unref(macio, pci_bus, &error_fatal);
 
+pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
 for (i = 0; i < 4; i++) {
 qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
   qdev_get_gpio_in(pic_dev, 0x1b + i));
@@ -407,6 +394,16 @@ static void ppc_core99_init(MachineState *machine)
 }
 }
 
+/* OpenPIC */
+s = SYS_BUS_DEVICE(pic_dev);
+k = 0;
+for (i = 0; i < smp_cpus; i++) {
+for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
+sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
+}
+}
+g_free(openpic_irqs);
+
 /* We only emulate 2 out of 3 IDE controllers for now */
 ide_drive_get(hd, ARRAY_SIZE(hd));
 
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 707dfab50c..6c05f3bfd2 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -115,7 +115,7 @@ struct NewWorldMacIOState {
 
 bool has_pmu;
 bool has_adb;
-OpenPICState *pic;
+OpenPICS

[PATCH v2 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine

In order to move the OpenPIC device to the macio device, the PCI bus needs to be
initialised before the macio device and also before wiring the OpenPIC IRQs.

Signed-off-by: Mark Cave-Ayland 
---
 hw/ppc/mac_newworld.c | 46 ---
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index c0accda592..708bb2f1ab 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -139,6 +139,7 @@ static void ppc_core99_init(MachineState *machine)
 int machine_arch;
 SysBusDevice *s;
 DeviceState *dev, *pic_dev;
+DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
 hwaddr nvram_addr = 0xFFF04000;
 uint64_t tbfreq;
 unsigned int smp_cpus = machine->smp.cpus;
@@ -320,35 +321,24 @@ static void ppc_core99_init(MachineState *machine)
 sysbus_mmio_map(s, 0, 0xf080);
 sysbus_mmio_map(s, 1, 0xf0c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 machine_arch = ARCH_MAC99_U3;
 } else {
 /* Use values found on a real PowerMac */
 /* Uninorth AGP bus */
-dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
-s = SYS_BUS_DEVICE(dev);
+uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
+s = SYS_BUS_DEVICE(uninorth_agp_dev);
 sysbus_realize_and_unref(s, &error_fatal);
 sysbus_mmio_map(s, 0, 0xf080);
 sysbus_mmio_map(s, 1, 0xf0c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 /* Uninorth internal bus */
-dev = qdev_new(TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
-s = SYS_BUS_DEVICE(dev);
+uninorth_internal_dev = qdev_new(
+TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
+s = SYS_BUS_DEVICE(uninorth_internal_dev);
 sysbus_realize_and_unref(s, &error_fatal);
 sysbus_mmio_map(s, 0, 0xf480);
 sysbus_mmio_map(s, 1, 0xf4c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 /* Uninorth main bus */
 dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
 qdev_prop_set_uint32(dev, "ofw-addr", 0xf200);
@@ -364,10 +354,6 @@ static void ppc_core99_init(MachineState *machine)
 sysbus_mmio_map(s, 0, 0xf280);
 sysbus_mmio_map(s, 1, 0xf2c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 machine_arch = ARCH_MAC99;
 }
 
@@ -401,6 +387,26 @@ static void ppc_core99_init(MachineState *machine)
 
 pci_realize_and_unref(macio, pci_bus, &error_fatal);
 
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
+  qdev_get_gpio_in(pic_dev, 0x1b + i));
+}
+
+/* TODO: additional PCI buses only wired up for 32-bit machines */
+if (PPC_INPUT(env) != PPC_FLAGS_INPUT_970) {
+/* Uninorth AGP bus */
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(uninorth_agp_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x1b + i));
+}
+
+/* Uninorth internal bus */
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(uninorth_internal_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x1b + i));
+}
+}
+
 /* We only emulate 2 out of 3 IDE controllers for now */
 ide_drive_get(hd, ARRAY_SIZE(hd));
 
-- 
2.20.1

[PATCH v2 7/7] macio: don't set user_creatable to false

Now that all of the object property links to the heathrow PIC and OpenPIC have
been removed from the macio devices, it is safe to allow the macio-oldworld
and macio-neworld devices to be marked as user_creatable.

Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/macio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index d17cffd868..e6eeb575d5 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -457,8 +457,6 @@ static void macio_class_init(ObjectClass *klass, void *data)
 k->class_id = PCI_CLASS_OTHERS << 8;
 device_class_set_props(dc, macio_properties);
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-/* Reason: requires PIC property links to be set in macio_*_realize() */
-dc->user_creatable = false;
 }
 
 static const TypeInfo macio_bus_info = {
-- 
2.20.1

[PATCH v2 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env)

This condition will have already been caught when wiring the heathrow PIC
IRQs to the CPU.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: David Gibson 
---
 hw/ppc/mac_oldworld.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 04f98a4d81..2ead34bdf1 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -251,12 +251,6 @@ static void ppc_heathrow_init(MachineState *machine)
 tbfreq = TBFREQ;
 }
 
-/* init basic PC hardware */
-if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
-error_report("Only 6xx bus is supported on heathrow machine");
-exit(1);
-}
-
 /* Grackle PCI host bridge */
 dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
 qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-- 
2.20.1

[PATCH v2 3/7] macio: move heathrow PIC inside macio-oldworld device

The heathrow PIC is located within the macio device on real hardware so make it
a child of the macio-oldworld device. This also removes the need for setting and
checking a separate PIC object property link on the macio-oldworld device which
currently causes the automated QOM introspection tests to fail.

Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/macio.c | 20 +--
 hw/ppc/mac_oldworld.c | 66 +--
 include/hw/misc/macio/macio.h |  2 +-
 3 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index bb601f782c..cfb87da6c9 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -140,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 {
 MacIOState *s = MACIO(d);
 OldWorldMacIOState *os = OLDWORLD_MACIO(d);
-DeviceState *pic_dev = DEVICE(os->pic);
+DeviceState *pic_dev = DEVICE(&os->pic);
 Error *err = NULL;
 SysBusDevice *sysbus_dev;
 
@@ -150,6 +150,14 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 return;
 }
 
+/* Heathrow PIC */
+if (!qdev_realize(DEVICE(&os->pic), BUS(&s->macio_bus), errp)) {
+return;
+}
+sysbus_dev = SYS_BUS_DEVICE(&os->pic);
+memory_region_add_subregion(&s->bar, 0x0,
+sysbus_mmio_get_region(sysbus_dev, 0));
+
 qdev_prop_set_uint64(DEVICE(&s->cuda), "timebase-frequency",
  s->frequency);
 if (!qdev_realize(DEVICE(&s->cuda), BUS(&s->macio_bus), errp)) {
@@ -175,11 +183,6 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 sysbus_mmio_get_region(sysbus_dev, 0));
 pmac_format_nvram_partition(&os->nvram, os->nvram.size);
 
-/* Heathrow PIC */
-sysbus_dev = SYS_BUS_DEVICE(os->pic);
-memory_region_add_subregion(&s->bar, 0x0,
-sysbus_mmio_get_region(sysbus_dev, 0));
-
 /* IDE buses */
 macio_realize_ide(s, &os->ide[0],
   qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
@@ -218,10 +221,7 @@ static void macio_oldworld_init(Object *obj)
 DeviceState *dev;
 int i;
 
-object_property_add_link(obj, "pic", TYPE_HEATHROW,
- (Object **) &os->pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+object_initialize_child(OBJECT(s), "pic", &os->pic, TYPE_HEATHROW);
 
 object_initialize_child(OBJECT(s), "cuda", &s->cuda, TYPE_CUDA);
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index e58e0525fe..44ee99be88 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
 MACIOIDEState *macio_ide;
 ESCCState *escc;
 SysBusDevice *s;
-DeviceState *dev, *pic_dev;
+DeviceState *dev, *pic_dev, *grackle_dev;
 BusState *adb_bus;
 uint64_t bios_addr;
 int bios_size;
@@ -227,10 +227,17 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 }
 
+/* Timebase Frequency */
+if (kvm_enabled()) {
+tbfreq = kvmppc_get_tbfreq();
+} else {
+tbfreq = TBFREQ;
+}
+
 /* Grackle PCI host bridge */
-dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-s = SYS_BUS_DEVICE(dev);
+grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x8000);
+s = SYS_BUS_DEVICE(grackle_dev);
 sysbus_realize_and_unref(s, &error_fatal);
 
 sysbus_mmio_map(s, 0, GRACKLE_BASE);
@@ -242,14 +249,30 @@ static void ppc_heathrow_init(MachineState *machine)
 memory_region_add_subregion(get_system_memory(), 0xfe00,
 sysbus_mmio_get_region(s, 3));
 
-/* XXX: we register only 1 output pin for heathrow PIC */
-pic_dev = qdev_new(TYPE_HEATHROW);
-sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), &error_fatal);
+pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
+
+/* MacIO */
+macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
+dev = DEVICE(macio);
+qdev_prop_set_uint64(dev, "frequency", tbfreq);
+
+escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
+pci_realize_and_unref(macio, pci_bus, &error_fatal);
+
+pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(grackle_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x15 + i));
+}
 
 /* Connect the heathrow PIC outputs to the 6xx bus */
 for (i = 0; i < smp_cpus; i++) {
 switch (PPC_INPUT(env)) {
 case PPC_FLAGS_INPUT_6xx:
+/* XXX: we register only 1 output p

Re: [PATCH] Deprecate pmem=on with non-DAX capable backend file

On 12/29/20 6:29 PM, Igor Mammedov wrote:
> It is not safe to pretend that emulated NVDIMM supports
> persistence while backend actually failed to enable it
> and used non-persistent mapping as fall back.
> Instead of falling-back, QEMU should be more strict and
> error out with clear message that it's not supported.
> So if user asks for persistence (pmem=on), they should
> store backing file on NVDIMM.
> 
> Signed-off-by: Igor Mammedov 
> ---
>  docs/system/deprecated.rst | 14 ++
>  util/mmap-alloc.c  |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index bacd76d7a5..ba4f6ed2fe 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -327,6 +327,20 @@ The Raspberry Pi machines come in various models (A, A+, 
> B, B+). To be able
>  to distinguish which model QEMU is implementing, the ``raspi2`` and 
> ``raspi3``
>  machines have been renamed ``raspi2b`` and ``raspi3b``.
>  
> +Backend options
> +---
> +
> +Using non-persistent backing file with pmem=on (since 6.0)
> +''
> +
> +This option is used when ``memory-backend-file`` is consumed by emulated 
> NVDIMM
> +device. However enabling ``memory-backend-file.pmem`` option, when backing 
> file
> +is not DAX capable or not on a filesystem that support direct mapping of 
> persistent

Maybe clearer enumerating? As:
"is a) not DAX capable or b) not on a filesystem that support direct
mapping of persistent"

> +memory, is not safe and may lead to data loss or corruption in case of host 
> crash.
> +Using pmem=on option with such file will return error, instead of a warning.

Not sure the difference between warn/err is important in the doc.

> +Options are to move backing file to NVDIMM storage or modify VM configuration
> +to set ``pmem=off`` to continue using fake NVDIMM without persistence 
> guaranties.

Maybe:

The possibilities to continue using fake NVDIMM (without persistence
guaranties) are:
- move backing file to NVDIMM storage
- modify VM configuration to set ``pmem=off``

> +
>  Device options
>  --
>  
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 27dcccd8ec..d226273a98 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -20,6 +20,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/mmap-alloc.h"
>  #include "qemu/host-utils.h"
> +#include "qemu/error-report.h"
>  
>  #define HUGETLBFS_MAGIC   0x958458f6
>  
> @@ -166,6 +167,8 @@ void *qemu_ram_mmap(int fd,
>  "crash.\n", file_name);
>  g_free(proc_link);
>  g_free(file_name);
> +warn_report("Deprecated using non DAX backing file with"
> +" pmem=on option");

Maybe "Using non DAX backing file with 'pmem=on' option is deprecated"?

Beside the nitpicking comments,
Reviewed-by: Philippe Mathieu-Daudé 

>  }
>  /*
>   * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC,
>

[Bug 1908551] Re: aarch64 SVE emulation breaks strnlen and strrchr

I don't know why the test worked previously, and I did not
investigate, but as far as I can tell, the test is broken.

The test is returning a value >= maxlen because it it using
the wrong increment.  Fixed thus.


** Patch added: "z.patch"
   
https://bugs.launchpad.net/qemu/+bug/1908551/+attachment/5447723/+files/z.patch

** Changed in: qemu
   Status: Confirmed => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1908551

Title:
  aarch64 SVE emulation breaks strnlen and strrchr

Status in QEMU:
  Invalid

Bug description:
  arm optimized-routines have sve string functions with test code.

  the test worked up until recently: with qemu-5.2.0 i see

  $ qemu-aarch64 build/bin/test/strnlen
  PASS strnlen
  PASS __strnlen_aarch64
  __strnlen_aarch64_sve (0x490fa0, 32) len 32 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80"
  __strnlen_aarch64_sve (0x490fa0, 32) len 33 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a"
  __strnlen_aarch64_sve (0x490fa0, 33) len 33 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a"
  __strnlen_aarch64_sve (0x490fa0, 32) len 34 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 33) len 34 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 34) len 34 returned 64, expected 34
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 32) len 35 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a\x00c"
  __strnlen_aarch64_sve (0x490fa0, 33) len 35 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab\x00"
  __strnlen_aarch64_sve (0x490fa0, 34) len 35 returned 64, expected 34
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80abc"
  __strnlen_aarch64_sve (0x490fa0, 35) len 35 returned 64, expected 35
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80abc"
  FAIL __strnlen_aarch64_sve

  however the test passes with

  qemu-aarch64 -cpu max,sve-max-vq=2

  there should be nothing vector length specific in the code.

  i haven't debugged it further, to reproduce the issue clone
  https://github.com/ARM-software/optimized-routines

  and run 'make build/bin/test/strnlen' with a config.mk like

  SUBS = string
  ARCH = aarch64
  CROSS_COMPILE = aarch64-none-linux-gnu-
  CC = $(CROSS_COMPILE)gcc
  CFLAGS = -std=c99 -pipe -O3
  CFLAGS += -march=armv8.2-a+sve
  EMULATOR = qemu-aarch64

  (native compilation works too, and you can run 'make check' to
  run all string tests) this will build a static linked executable
  into build/bin/test. if you want a smaller test case edit
  string/test/strnlen.c

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1908551/+subscriptions

[Bug 1908551] Re: aarch64 SVE emulation breaks strnlen and strrchr

FWIW, as I think on this further, this probably isn't the
ideal fix -- I recall now that INCP is a "reduction" class
instruction and thus its overhead is non-trivial.

We could instead add an integer min operation at label 9,
which is outside of the main loop.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1908551

Title:
  aarch64 SVE emulation breaks strnlen and strrchr

Status in QEMU:
  Invalid

Bug description:
  arm optimized-routines have sve string functions with test code.

  the test worked up until recently: with qemu-5.2.0 i see

  $ qemu-aarch64 build/bin/test/strnlen
  PASS strnlen
  PASS __strnlen_aarch64
  __strnlen_aarch64_sve (0x490fa0, 32) len 32 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80"
  __strnlen_aarch64_sve (0x490fa0, 32) len 33 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a"
  __strnlen_aarch64_sve (0x490fa0, 33) len 33 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a"
  __strnlen_aarch64_sve (0x490fa0, 32) len 34 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 33) len 34 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 34) len 34 returned 64, expected 34
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 32) len 35 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a\x00c"
  __strnlen_aarch64_sve (0x490fa0, 33) len 35 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab\x00"
  __strnlen_aarch64_sve (0x490fa0, 34) len 35 returned 64, expected 34
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80abc"
  __strnlen_aarch64_sve (0x490fa0, 35) len 35 returned 64, expected 35
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80abc"
  FAIL __strnlen_aarch64_sve

  however the test passes with

  qemu-aarch64 -cpu max,sve-max-vq=2

  there should be nothing vector length specific in the code.

  i haven't debugged it further, to reproduce the issue clone
  https://github.com/ARM-software/optimized-routines

  and run 'make build/bin/test/strnlen' with a config.mk like

  SUBS = string
  ARCH = aarch64
  CROSS_COMPILE = aarch64-none-linux-gnu-
  CC = $(CROSS_COMPILE)gcc
  CFLAGS = -std=c99 -pipe -O3
  CFLAGS += -march=armv8.2-a+sve
  EMULATOR = qemu-aarch64

  (native compilation works too, and you can run 'make check' to
  run all string tests) this will build a static linked executable
  into build/bin/test. if you want a smaller test case edit
  string/test/strnlen.c

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1908551/+subscriptions

Re: [PATCH] gitlab-ci.yml: Add openSUSE Leap 15.2 for gitlab CI/CD

2020-12-29 Thread Wainer dos Santos Moschetta




On 12/29/20 5:50 AM, Cho, Yu-Chen wrote:

Add build-system-opensuse jobs and opensuse-leap.docker dockerfile.
Use openSUSE Leap 15.2 container image in the gitlab-CI.

Signed-off-by: Cho, Yu-Chen 
---
v4:
Add package tar and fix python3 version from 3.8 to 3.6
Add acceptance-system-opensuse back.

v3:
Drop the "acceptance-system-opensuse" job part of the
patch for now to get at least the basic compile-coverage

v2:
Drop some package from dockerfile to make docker image more light.

v1:
Add build-system-opensuse jobs and opensuse-leap.docker dockerfile.
Use openSUSE Leap 15.2 container image in the gitlab-CI.
---
  .gitlab-ci.d/containers.yml   |  5 ++
  .gitlab-ci.yml| 31 +++
  tests/docker/dockerfiles/opensuse-leap.docker | 55 +++
  3 files changed, 91 insertions(+)
  create mode 100644 tests/docker/dockerfiles/opensuse-leap.docker

diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index 892ca8d838..910754a699 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -246,3 +246,8 @@ amd64-ubuntu-container:
<<: *container_job_definition
variables:
  NAME: ubuntu
+
+amd64-opensuse-leap-container:
+  <<: *container_job_definition
+  variables:
+NAME: opensuse-leap
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 98bff03b47..fb65a2d29c 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -195,6 +195,37 @@ acceptance-system-centos:
  MAKE_CHECK_ARGS: check-acceptance
<<: *acceptance_definition
  
+build-system-opensuse:

+  <<: *native_build_job_definition
+  variables:
+IMAGE: opensuse-leap
+TARGETS: s390x-softmmu x86_64-softmmu aarch64-softmmu
+MAKE_CHECK_ARGS: check-build
+  artifacts:
+expire_in: 2 days
+paths:
+  - build
+
+check-system-opensuse:
+  <<: *native_test_job_definition
+  needs:
+- job: build-system-opensuse
+  artifacts: true
+  variables:
+IMAGE: opensuse-leap
+MAKE_CHECK_ARGS: check
+
+acceptance-system-opensuse:
+   <<: *native_test_job_definition
+   needs:
+ - job: build-system-opensuse
+   artifacts: true
+   variables:
+ IMAGE: opensuse-leap
+ MAKE_CHECK_ARGS: check-acceptance
+   <<: *acceptance_definition
+
+
  build-disabled:
<<: *native_build_job_definition
variables:
diff --git a/tests/docker/dockerfiles/opensuse-leap.docker 
b/tests/docker/dockerfiles/opensuse-leap.docker
new file mode 100644
index 00..0e64893e4a
--- /dev/null
+++ b/tests/docker/dockerfiles/opensuse-leap.docker
@@ -0,0 +1,55 @@
+FROM opensuse/leap:15.2
+
+# Please keep this list sorted alphabetically


Except for the list of packages below not being sorted below, this 
version of the patch looks good to me. So,


Reviewed-by: Wainer dos Santos Moschetta 

I did some tests locally, it is working. Also on Gitlab CI all the new 
builds pass:


https://gitlab.com/wainersm/qemu/-/pipelines/235534062

So,

Tested-by: Wainer dos Santos Moschetta 


+ENV PACKAGES \
+bc \
+brlapi-devel \
+bzip2 \
+cyrus-sasl-devel \
+gcc \
+gcc-c++ \
+mkisofs \
+gettext-runtime \
+git \
+glib2-devel \
+glusterfs-devel \
+libgnutls-devel \
+gtk3-devel \
+libaio-devel \
+libattr-devel \
+libcap-ng-devel \
+libepoxy-devel \
+libfdt-devel \
+libiscsi-devel \
+libjpeg8-devel \
+libpmem-devel \
+libpng16-devel \
+librbd-devel \
+libseccomp-devel \
+libssh-devel \
+lzo-devel \
+make \
+libSDL2_image-devel \
+ncurses-devel \
+ninja \
+libnuma-devel \
+perl \
+libpixman-1-0-devel \
+python3-base \
+python3-virtualenv \
+rdma-core-devel \
+libSDL2-devel \
+snappy-devel \
+libspice-server-devel \
+systemd-devel \
+systemtap-sdt-devel \
+tar \
+usbredir-devel \
+virglrenderer-devel \
+xen-devel \
+vte-devel \
+zlib-devel
+ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3.6
+
+RUN zypper update -y && zypper --non-interactive install -y $PACKAGES
+RUN rpm -q $PACKAGES | sort > /packages.txt

[Bug 1908551] Re: aarch64 SVE emulation breaks strnlen and strrchr

Bah.  The code at label 9 does not match the comment.
Best fixed thus.

** Patch added: "z.patch"
   
https://bugs.launchpad.net/qemu/+bug/1908551/+attachment/5447736/+files/z.patch

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1908551

Title:
  aarch64 SVE emulation breaks strnlen and strrchr

Status in QEMU:
  Invalid

Bug description:
  arm optimized-routines have sve string functions with test code.

  the test worked up until recently: with qemu-5.2.0 i see

  $ qemu-aarch64 build/bin/test/strnlen
  PASS strnlen
  PASS __strnlen_aarch64
  __strnlen_aarch64_sve (0x490fa0, 32) len 32 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80"
  __strnlen_aarch64_sve (0x490fa0, 32) len 33 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a"
  __strnlen_aarch64_sve (0x490fa0, 33) len 33 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a"
  __strnlen_aarch64_sve (0x490fa0, 32) len 34 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 33) len 34 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 34) len 34 returned 64, expected 34
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab"
  __strnlen_aarch64_sve (0x490fa0, 32) len 35 returned 64, expected 32
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80a\x00c"
  __strnlen_aarch64_sve (0x490fa0, 33) len 35 returned 64, expected 33
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80ab\x00"
  __strnlen_aarch64_sve (0x490fa0, 34) len 35 returned 64, expected 34
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80abc"
  __strnlen_aarch64_sve (0x490fa0, 35) len 35 returned 64, expected 35
  input: "abcdefghijklmnopqrstuvwxyz\{|}~\x7f\x80abc"
  FAIL __strnlen_aarch64_sve

  however the test passes with

  qemu-aarch64 -cpu max,sve-max-vq=2

  there should be nothing vector length specific in the code.

  i haven't debugged it further, to reproduce the issue clone
  https://github.com/ARM-software/optimized-routines

  and run 'make build/bin/test/strnlen' with a config.mk like

  SUBS = string
  ARCH = aarch64
  CROSS_COMPILE = aarch64-none-linux-gnu-
  CC = $(CROSS_COMPILE)gcc
  CFLAGS = -std=c99 -pipe -O3
  CFLAGS += -march=armv8.2-a+sve
  EMULATOR = qemu-aarch64

  (native compilation works too, and you can run 'make check' to
  run all string tests) this will build a static linked executable
  into build/bin/test. if you want a smaller test case edit
  string/test/strnlen.c

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1908551/+subscriptions

Re: [PATCH] Deprecate pmem=on with non-DAX capable backend file

On Tue, 29 Dec 2020 19:04:58 +0100
Philippe Mathieu-Daudé  wrote:

> On 12/29/20 6:29 PM, Igor Mammedov wrote:
> > It is not safe to pretend that emulated NVDIMM supports
> > persistence while backend actually failed to enable it
> > and used non-persistent mapping as fall back.
> > Instead of falling-back, QEMU should be more strict and
> > error out with clear message that it's not supported.
> > So if user asks for persistence (pmem=on), they should
> > store backing file on NVDIMM.
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  docs/system/deprecated.rst | 14 ++
> >  util/mmap-alloc.c  |  3 +++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> > index bacd76d7a5..ba4f6ed2fe 100644
> > --- a/docs/system/deprecated.rst
> > +++ b/docs/system/deprecated.rst
> > @@ -327,6 +327,20 @@ The Raspberry Pi machines come in various models (A, 
> > A+, B, B+). To be able
> >  to distinguish which model QEMU is implementing, the ``raspi2`` and 
> > ``raspi3``
> >  machines have been renamed ``raspi2b`` and ``raspi3b``.
> >  
> > +Backend options
> > +---
> > +
> > +Using non-persistent backing file with pmem=on (since 6.0)
> > +''
> > +
> > +This option is used when ``memory-backend-file`` is consumed by emulated 
> > NVDIMM
> > +device. However enabling ``memory-backend-file.pmem`` option, when backing 
> > file
> > +is not DAX capable or not on a filesystem that support direct mapping of 
> > persistent  
> 
> Maybe clearer enumerating? As:
> "is a) not DAX capable or b) not on a filesystem that support direct
> mapping of persistent"

will change it to your variant in v2

> 
> > +memory, is not safe and may lead to data loss or corruption in case of 
> > host crash.
> > +Using pmem=on option with such file will return error, instead of a 
> > warning.  
> 
> Not sure the difference between warn/err is important in the doc.
not many care about warnings until QEMU starts fine,
I've mentioned error here so that whomever reading this would know what to 
expect

> 
> > +Options are to move backing file to NVDIMM storage or modify VM 
> > configuration
> > +to set ``pmem=off`` to continue using fake NVDIMM without persistence 
> > guaranties.  
> 
> Maybe:
> 
> The possibilities to continue using fake NVDIMM (without persistence
> guaranties) are:
> - move backing file to NVDIMM storage
> - modify VM configuration to set ``pmem=off``

only the later is faking nvdimm, the first is properly emulated one with 
persistence guaranties.
Maybe:
 Options are:
- modify VM configuration to set ``pmem=off`` to continue using fake NVDIMM
  (without persistence guaranties) on with backing file on non DAX storage
- move backing file to NVDIMM storage and keep ``pmem=on``,
  to have NVDIMM with persistence guaranties.

> > +
> >  Device options
> >  --
> >  
> > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> > index 27dcccd8ec..d226273a98 100644
> > --- a/util/mmap-alloc.c
> > +++ b/util/mmap-alloc.c
> > @@ -20,6 +20,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/mmap-alloc.h"
> >  #include "qemu/host-utils.h"
> > +#include "qemu/error-report.h"
> >  
> >  #define HUGETLBFS_MAGIC   0x958458f6
> >  
> > @@ -166,6 +167,8 @@ void *qemu_ram_mmap(int fd,
> >  "crash.\n", file_name);
> >  g_free(proc_link);
> >  g_free(file_name);
> > +warn_report("Deprecated using non DAX backing file with"
> > +" pmem=on option");  
> 
> Maybe "Using non DAX backing file with 'pmem=on' option is deprecated"?
ok

> 
> Beside the nitpicking comments,
> Reviewed-by: Philippe Mathieu-Daudé 
> 
> >  }
> >  /*
> >   * if map failed with MAP_SHARED_VALIDATE | MAP_SYNC,
> >   
>

[Bug 1909392] Re: qemu-arm crashes (SIGSEGV) when executing push instruction

The program is buggy.

The first instruction sets the stack to 0x2002,
but that address is not mapped.

Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD   0x01 0x0800 0x0800 0x0025c 0x0025c R E 0x1
  LOAD   0x02 0x2000 0x0800025c 0x0 0x00600 RW  0x1

The data segment only goes from 0x2000 - 0x2600.

** Changed in: qemu
   Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1909392

Title:
  qemu-arm crashes (SIGSEGV) when executing push instruction

Status in QEMU:
  Invalid

Bug description:
  Dear all,
  I am afraid I found a problem, it seems like qemu-arm crashes when executing 
assembly push instruction.
  I use qemu version 5.2.0, but it checked an older version (4.2.1) and the 
problem was also present. I start qemu using "qemu-arm -cpu cortex-m4 
-singlestep -g 1234 "
  Callstack before crash (host)
  #0  0x5575961f in stl_he_p (ptr=0x2002fffc, v=0) at 
/home/faust1002/Programming/qemu/qemu-5.2.0/include/qemu/bswap.h:353
  #1  0x55759716 in stl_le_p (ptr=0x2002fffc, v=0) at 
/home/faust1002/Programming/qemu/qemu-5.2.0/include/qemu/bswap.h:395
  #2  0x5575d3c3 in tcg_qemu_tb_exec (env=0x55d28050, 
tb_ptr=0x7fffe800010a "\r\b") at ../tcg/tci.c:1221
  #3  0x556bd982 in cpu_tb_exec (cpu=0x55d1fd70, 
itb=0x7fffe800) at ../accel/tcg/cpu-exec.c:178
  #4  0x556be57e in cpu_loop_exec_tb (cpu=0x55d1fd70, 
tb=0x7fffe800, last_tb=0x7fffd8a8, tb_exit=0x7fffd8a0) at 
../accel/tcg/cpu-exec.c:658
  #5  0x556be7ea in cpu_exec (cpu=0x55d1fd70) at 
../accel/tcg/cpu-exec.c:771
  #6  0x5560af1d in cpu_loop (env=0x55d28050) at 
../linux-user/arm/cpu_loop.c:237
  #7  0x557415a7 in main (argc=7, argv=0x7fffe0f8, 
envp=0x7fffe138) at ../linux-user/main.c:861
  Callstack before crash (target)
  Program received signal SIGSEGV, Segmentation fault.
  Reset_Handler () at startup.s:48
  48push {r14}
  Please find the elf file I use attached.
  Kind regards

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1909392/+subscriptions

[PATCH 0/7] vt82c686b clean ups - part II

This is continuing cleaning up the vt82c686b model with the aim to add
vt8231 emulation without too much duplication, reusing or fixing the
existing model where possible. (DISCALIMER: It does not aim to fix all
existing bugs or make the model perfectly emulate the real chip.) This
series continues general clean up and finishes the power management
part. It is based on previous series and also needs Bonito fix
(currently tested with Jiaxun's proposal).

Still to go are superio and isa bridge parts that will be addressed in
some later series but this and previous series could be merged so far
independently of those future series so I'd appreciate if this could
be reviewed and merged to keep outstanding patches managable.

I can submit this and previous series together as one series if that
helps.

Regards,
BALATON Zoltan

BALATON Zoltan (7):
  vt82c686: Use shorter name for local variable holding object state
  vt82c686: Rename superio config related parts
  vt82c686: Move superio memory region to SuperIOConfig struct
  vt82c686: Reorganise code
  vt82c686: Fix SMBus IO base and configuration registers
  vt82c686: Fix up power management io base and config
  vt82c686: Make vt82c686b-pm an abstract base class and add vt8231-pm
based on it

 hw/isa/trace-events   |   2 +
 hw/isa/vt82c686.c | 417 ++
 hw/mips/fuloong2e.c   |   4 +-
 include/hw/isa/vt82c686.h |   1 +
 4 files changed, 251 insertions(+), 173 deletions(-)

-- 
2.21.3

[PATCH 2/7] vt82c686: Rename superio config related parts

Use less confusing naming for superio config register handling related
parts that makes it clearer what belongs to this part.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/vt82c686.c | 48 +++
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 2633cfe7dc..a6f5a0843d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -27,7 +27,7 @@
 #include "trace.h"
 
 typedef struct SuperIOConfig {
-uint8_t config[0x100];
+uint8_t regs[0x100];
 uint8_t index;
 uint8_t data;
 } SuperIOConfig;
@@ -35,23 +35,23 @@ typedef struct SuperIOConfig {
 struct VT82C686BISAState {
 PCIDevice dev;
 MemoryRegion superio;
-SuperIOConfig superio_conf;
+SuperIOConfig superio_cfg;
 };
 
 OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
 
-static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
-  unsigned size)
+static void superio_cfg_write(void *opaque, hwaddr addr, uint64_t data,
+  unsigned size)
 {
-SuperIOConfig *superio_conf = opaque;
+SuperIOConfig *sc = opaque;
 
 if (addr == 0x3f0) { /* config index register */
-superio_conf->index = data & 0xff;
+sc->index = data & 0xff;
 } else {
 bool can_write = true;
 /* 0x3f1, config data register */
-trace_via_superio_write(superio_conf->index, data & 0xff);
-switch (superio_conf->index) {
+trace_via_superio_write(sc->index, data & 0xff);
+switch (sc->index) {
 case 0x00 ... 0xdf:
 case 0xe4:
 case 0xe5:
@@ -69,23 +69,23 @@ static void superio_ioport_writeb(void *opaque, hwaddr 
addr, uint64_t data,
 
 }
 if (can_write) {
-superio_conf->config[superio_conf->index] = data & 0xff;
+sc->regs[sc->index] = data & 0xff;
 }
 }
 }
 
-static uint64_t superio_ioport_readb(void *opaque, hwaddr addr, unsigned size)
+static uint64_t superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
 {
-SuperIOConfig *superio_conf = opaque;
-uint8_t val = superio_conf->config[superio_conf->index];
+SuperIOConfig *sc = opaque;
+uint8_t val = sc->regs[sc->index];
 
-trace_via_superio_read(superio_conf->index, val);
+trace_via_superio_read(sc->index, val);
 return val;
 }
 
-static const MemoryRegionOps superio_ops = {
-.read = superio_ioport_readb,
-.write = superio_ioport_writeb,
+static const MemoryRegionOps superio_cfg_ops = {
+.read = superio_cfg_read,
+.write = superio_cfg_write,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .impl = {
 .min_access_size = 1,
@@ -112,12 +112,12 @@ static void vt82c686b_isa_reset(DeviceState *dev)
 pci_conf[0x5f] = 0x04;
 pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
 
-s->superio_conf.config[0xe0] = 0x3c;
-s->superio_conf.config[0xe2] = 0x03;
-s->superio_conf.config[0xe3] = 0xfc;
-s->superio_conf.config[0xe6] = 0xde;
-s->superio_conf.config[0xe7] = 0xfe;
-s->superio_conf.config[0xe8] = 0xbe;
+s->superio_cfg.regs[0xe0] = 0x3c; /* Device ID */
+s->superio_cfg.regs[0xe2] = 0x03; /* Function select */
+s->superio_cfg.regs[0xe3] = 0xfc; /* Floppy ctrl base addr */
+s->superio_cfg.regs[0xe6] = 0xde; /* Parallel port base addr */
+s->superio_cfg.regs[0xe7] = 0xfe; /* Serial port 1 base addr */
+s->superio_cfg.regs[0xe8] = 0xbe; /* Serial port 2 base addr */
 }
 
 /* write config pci function0 registers. PCI-ISA bridge */
@@ -311,8 +311,8 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
 }
 }
 
-memory_region_init_io(&s->superio, OBJECT(d), &superio_ops,
-  &s->superio_conf, "superio", 2);
+memory_region_init_io(&s->superio, OBJECT(d), &superio_cfg_ops,
+  &s->superio_cfg, "superio", 2);
 memory_region_set_enabled(&s->superio, false);
 /*
  * The floppy also uses 0x3f0 and 0x3f1.
-- 
2.21.3

[PATCH 5/7] vt82c686: Fix SMBus IO base and configuration registers

The base address of the SMBus io ports and its enabled status is set
by registers in the PCI config space but this was not correctly
emulated. Instead the SMBus registers were mapped on realize to the
base address set by a property to the address expected by fuloong2e
firmware.

Fix the base and config register handling to more closely model
hardware which allows to remove the property and allows the guest to
control this mapping. Do all this in reset instead of realize so it's
correctly updated on reset.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/vt82c686.c   | 49 +
 hw/mips/fuloong2e.c |  4 +---
 2 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index fe8961b057..9c4d153022 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -22,6 +22,7 @@
 #include "hw/i2c/pm_smbus.h"
 #include "qapi/error.h"
 #include "qemu/module.h"
+#include "qemu/range.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
 #include "trace.h"
@@ -34,7 +35,6 @@ struct VT686PMState {
 ACPIREGS ar;
 APMState apm;
 PMSMBus smb;
-uint32_t smb_io_base;
 };
 
 static void pm_io_space_update(VT686PMState *s)
@@ -50,11 +50,22 @@ static void pm_io_space_update(VT686PMState *s)
 memory_region_transaction_commit();
 }
 
+static void smb_io_space_update(VT686PMState *s)
+{
+uint32_t smbase = pci_get_long(s->dev.config + 0x90) & 0xfff0UL;
+
+memory_region_transaction_begin();
+memory_region_set_address(&s->smb.io, smbase);
+memory_region_set_enabled(&s->smb.io, s->dev.config[0xd2] & BIT(0));
+memory_region_transaction_commit();
+}
+
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
 VT686PMState *s = opaque;
 
 pm_io_space_update(s);
+smb_io_space_update(s);
 return 0;
 }
 
@@ -77,8 +88,18 @@ static const VMStateDescription vmstate_acpi = {
 
 static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
 {
+VT686PMState *s = VT82C686B_PM(d);
+
 trace_via_pm_write(addr, val, len);
 pci_default_write_config(d, addr, val, len);
+if (ranges_overlap(addr, len, 0x90, 4)) {
+uint32_t v = pci_get_long(s->dev.config + 0x90);
+pci_set_long(s->dev.config + 0x90, (v & 0xfff0UL) | 1);
+}
+if (range_covers_byte(addr, len, 0xd2)) {
+s->dev.config[0xd2] &= 0xf;
+smb_io_space_update(s);
+}
 }
 
 static void pm_update_sci(VT686PMState *s)
@@ -103,6 +124,17 @@ static void pm_tmr_timer(ACPIREGS *ar)
 pm_update_sci(s);
 }
 
+static void vt82c686b_pm_reset(DeviceState *d)
+{
+VT686PMState *s = VT82C686B_PM(d);
+
+/* SMBus IO base */
+pci_set_long(s->dev.config + 0x90, 1);
+s->dev.config[0xd2] = 0;
+
+smb_io_space_update(s);
+}
+
 static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 {
 VT686PMState *s = VT82C686B_PM(dev);
@@ -116,13 +148,9 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error 
**errp)
 /* 0x48-0x4B is Power Management I/O Base */
 pci_set_long(pci_conf + 0x48, 0x0001);
 
-/* SMB ports:0xeee0~0xeeef */
-s->smb_io_base = ((s->smb_io_base & 0xfff0) + 0x0);
-pci_conf[0x90] = s->smb_io_base | 1;
-pci_conf[0x91] = s->smb_io_base >> 8;
-pci_conf[0xd2] = 0x90;
 pm_smbus_init(DEVICE(s), &s->smb, false);
-memory_region_add_subregion(get_system_io(), s->smb_io_base, &s->smb.io);
+memory_region_add_subregion(pci_address_space_io(dev), 0, &s->smb.io);
+memory_region_set_enabled(&s->smb.io, false);
 
 apm_init(dev, &s->apm, NULL, s);
 
@@ -135,11 +163,6 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error 
**errp)
 acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2);
 }
 
-static Property via_pm_properties[] = {
-DEFINE_PROP_UINT32("smb_io_base", VT686PMState, smb_io_base, 0),
-DEFINE_PROP_END_OF_LIST(),
-};
-
 static void via_pm_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -151,10 +174,10 @@ static void via_pm_class_init(ObjectClass *klass, void 
*data)
 k->device_id = PCI_DEVICE_ID_VIA_ACPI;
 k->class_id = PCI_CLASS_BRIDGE_OTHER;
 k->revision = 0x40;
+dc->reset = vt82c686b_pm_reset;
 dc->desc = "PM";
 dc->vmsd = &vmstate_acpi;
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-device_class_set_props(dc, via_pm_properties);
 }
 
 static const TypeInfo via_pm_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index f393509633..bf19b4603e 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -263,9 +263,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
 pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
 pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
 
-dev = pci_new(PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
-qdev_prop_set_uint32(DEVICE(dev), "smb_io_base", 0xeee1);
-pci_realize_and_unref(dev, pci_bus, &e

[PATCH 4/7] vt82c686: Reorganise code

Move lines around so that object definitions become consecutive and
not scattered around. This brings functions belonging to an object
together so it's clearer what's defined and what parts belong to which
object.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/vt82c686.c | 279 +++---
 1 file changed, 140 insertions(+), 139 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 30fe02f4c6..fe8961b057 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -26,112 +26,7 @@
 #include "exec/address-spaces.h"
 #include "trace.h"
 
-typedef struct SuperIOConfig {
-uint8_t regs[0x100];
-uint8_t index;
-MemoryRegion io;
-} SuperIOConfig;
-
-struct VT82C686BISAState {
-PCIDevice dev;
-SuperIOConfig superio_cfg;
-};
-
-OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
-
-static void superio_cfg_write(void *opaque, hwaddr addr, uint64_t data,
-  unsigned size)
-{
-SuperIOConfig *sc = opaque;
-
-if (addr == 0x3f0) { /* config index register */
-sc->index = data & 0xff;
-} else {
-bool can_write = true;
-/* 0x3f1, config data register */
-trace_via_superio_write(sc->index, data & 0xff);
-switch (sc->index) {
-case 0x00 ... 0xdf:
-case 0xe4:
-case 0xe5:
-case 0xe9 ... 0xed:
-case 0xf3:
-case 0xf5:
-case 0xf7:
-case 0xf9 ... 0xfb:
-case 0xfd ... 0xff:
-can_write = false;
-break;
-/* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
-default:
-break;
-
-}
-if (can_write) {
-sc->regs[sc->index] = data & 0xff;
-}
-}
-}
-
-static uint64_t superio_cfg_read(void *opaque, hwaddr addr, unsigned size)
-{
-SuperIOConfig *sc = opaque;
-uint8_t val = sc->regs[sc->index];
-
-trace_via_superio_read(sc->index, val);
-return val;
-}
-
-static const MemoryRegionOps superio_cfg_ops = {
-.read = superio_cfg_read,
-.write = superio_cfg_write,
-.endianness = DEVICE_NATIVE_ENDIAN,
-.impl = {
-.min_access_size = 1,
-.max_access_size = 1,
-},
-};
-
-static void vt82c686b_isa_reset(DeviceState *dev)
-{
-VT82C686BISAState *s = VT82C686B_ISA(dev);
-uint8_t *pci_conf = s->dev.config;
-
-pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
-pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
- PCI_COMMAND_MASTER | PCI_COMMAND_SPECIAL);
-pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
-
-pci_conf[0x48] = 0x01; /* Miscellaneous Control 3 */
-pci_conf[0x4a] = 0x04; /* IDE interrupt Routing */
-pci_conf[0x4f] = 0x03; /* DMA/Master Mem Access Control 3 */
-pci_conf[0x50] = 0x2d; /* PnP DMA Request Control */
-pci_conf[0x59] = 0x04;
-pci_conf[0x5a] = 0x04; /* KBC/RTC Control*/
-pci_conf[0x5f] = 0x04;
-pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
-
-s->superio_cfg.regs[0xe0] = 0x3c; /* Device ID */
-s->superio_cfg.regs[0xe2] = 0x03; /* Function select */
-s->superio_cfg.regs[0xe3] = 0xfc; /* Floppy ctrl base addr */
-s->superio_cfg.regs[0xe6] = 0xde; /* Parallel port base addr */
-s->superio_cfg.regs[0xe7] = 0xfe; /* Serial port 1 base addr */
-s->superio_cfg.regs[0xe8] = 0xbe; /* Serial port 2 base addr */
-}
-
-/* write config pci function0 registers. PCI-ISA bridge */
-static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
-   uint32_t val, int len)
-{
-VT82C686BISAState *s = VT82C686B_ISA(d);
-
-trace_via_isa_write(addr, val, len);
-pci_default_write_config(d, addr, val, len);
-if (addr == 0x85) {
-/* BIT(1): enable or disable superio config io ports */
-memory_region_set_enabled(&s->superio_cfg.io, val & BIT(1));
-}
-}
+OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
 
 struct VT686PMState {
 PCIDevice dev;
@@ -142,30 +37,6 @@ struct VT686PMState {
 uint32_t smb_io_base;
 };
 
-OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
-
-static void pm_update_sci(VT686PMState *s)
-{
-int sci_level, pmsts;
-
-pmsts = acpi_pm1_evt_get_sts(&s->ar);
-sci_level = (((pmsts & s->ar.pm1.evt.en) &
-  (ACPI_BITMASK_RT_CLOCK_ENABLE |
-   ACPI_BITMASK_POWER_BUTTON_ENABLE |
-   ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
-   ACPI_BITMASK_TIMER_ENABLE)) != 0);
-pci_set_irq(&s->dev, sci_level);
-/* schedule a timer interruption if needed */
-acpi_pm_tmr_update(&s->ar, (s->ar.pm1.evt.en & ACPI_BITMASK_TIMER_ENABLE) 
&&
-   !(pmsts & ACPI_BITMASK_TIMER_STATUS));
-}
-
-static void pm_tmr_timer(ACPIREGS *ar)
-{
-VT686PMState *s = container_of(ar, VT686PMState, ar);
-pm_update_sci(s);
-}
-
 static void pm_io_space_update(VT686PMState *s)
 {
 uint3

[PATCH 3/7] vt82c686: Move superio memory region to SuperIOConfig struct

The superio memory region holds the io space index/data registers used
to access the superio config registers that are implemented in struct
SuperIOConfig. To keep these related things together move the memory
region to SuperIOConfig and rename it accordingly.
Also remove the unused data member of SuperIOConfig which is not
needed as we store actual data values in the regs array.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/vt82c686.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index a6f5a0843d..30fe02f4c6 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -29,12 +29,11 @@
 typedef struct SuperIOConfig {
 uint8_t regs[0x100];
 uint8_t index;
-uint8_t data;
+MemoryRegion io;
 } SuperIOConfig;
 
 struct VT82C686BISAState {
 PCIDevice dev;
-MemoryRegion superio;
 SuperIOConfig superio_cfg;
 };
 
@@ -128,8 +127,9 @@ static void vt82c686b_write_config(PCIDevice *d, uint32_t 
addr,
 
 trace_via_isa_write(addr, val, len);
 pci_default_write_config(d, addr, val, len);
-if (addr == 0x85) {  /* enable or disable super IO configure */
-memory_region_set_enabled(&s->superio, val & 0x2);
+if (addr == 0x85) {
+/* BIT(1): enable or disable superio config io ports */
+memory_region_set_enabled(&s->superio_cfg.io, val & BIT(1));
 }
 }
 
@@ -311,15 +311,15 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
 }
 }
 
-memory_region_init_io(&s->superio, OBJECT(d), &superio_cfg_ops,
-  &s->superio_cfg, "superio", 2);
-memory_region_set_enabled(&s->superio, false);
+memory_region_init_io(&s->superio_cfg.io, OBJECT(d), &superio_cfg_ops,
+  &s->superio_cfg, "superio_cfg", 2);
+memory_region_set_enabled(&s->superio_cfg.io, false);
 /*
  * The floppy also uses 0x3f0 and 0x3f1.
  * But we do not emulate a floppy, so just set it here.
  */
 memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
-&s->superio);
+&s->superio_cfg.io);
 }
 
 static void via_class_init(ObjectClass *klass, void *data)
-- 
2.21.3

[PATCH 1/7] vt82c686: Use shorter name for local variable holding object state

Rename local variable holding object state for readability and
consistency.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/vt82c686.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 02d6759c00..2633cfe7dc 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -95,8 +95,8 @@ static const MemoryRegionOps superio_ops = {
 
 static void vt82c686b_isa_reset(DeviceState *dev)
 {
-VT82C686BISAState *vt82c = VT82C686B_ISA(dev);
-uint8_t *pci_conf = vt82c->dev.config;
+VT82C686BISAState *s = VT82C686B_ISA(dev);
+uint8_t *pci_conf = s->dev.config;
 
 pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
 pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
@@ -112,24 +112,24 @@ static void vt82c686b_isa_reset(DeviceState *dev)
 pci_conf[0x5f] = 0x04;
 pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
 
-vt82c->superio_conf.config[0xe0] = 0x3c;
-vt82c->superio_conf.config[0xe2] = 0x03;
-vt82c->superio_conf.config[0xe3] = 0xfc;
-vt82c->superio_conf.config[0xe6] = 0xde;
-vt82c->superio_conf.config[0xe7] = 0xfe;
-vt82c->superio_conf.config[0xe8] = 0xbe;
+s->superio_conf.config[0xe0] = 0x3c;
+s->superio_conf.config[0xe2] = 0x03;
+s->superio_conf.config[0xe3] = 0xfc;
+s->superio_conf.config[0xe6] = 0xde;
+s->superio_conf.config[0xe7] = 0xfe;
+s->superio_conf.config[0xe8] = 0xbe;
 }
 
 /* write config pci function0 registers. PCI-ISA bridge */
 static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
uint32_t val, int len)
 {
-VT82C686BISAState *vt686 = VT82C686B_ISA(d);
+VT82C686BISAState *s = VT82C686B_ISA(d);
 
 trace_via_isa_write(addr, val, len);
 pci_default_write_config(d, addr, val, len);
 if (addr == 0x85) {  /* enable or disable super IO configure */
-memory_region_set_enabled(&vt686->superio, val & 0x2);
+memory_region_set_enabled(&s->superio, val & 0x2);
 }
 }
 
@@ -289,7 +289,7 @@ static const VMStateDescription vmstate_via = {
 /* init the PCI-to-ISA bridge */
 static void vt82c686b_realize(PCIDevice *d, Error **errp)
 {
-VT82C686BISAState *vt82c = VT82C686B_ISA(d);
+VT82C686BISAState *s = VT82C686B_ISA(d);
 uint8_t *pci_conf;
 ISABus *isa_bus;
 uint8_t *wmask;
@@ -311,15 +311,15 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
 }
 }
 
-memory_region_init_io(&vt82c->superio, OBJECT(d), &superio_ops,
-  &vt82c->superio_conf, "superio", 2);
-memory_region_set_enabled(&vt82c->superio, false);
+memory_region_init_io(&s->superio, OBJECT(d), &superio_ops,
+  &s->superio_conf, "superio", 2);
+memory_region_set_enabled(&s->superio, false);
 /*
  * The floppy also uses 0x3f0 and 0x3f1.
  * But we do not emulate a floppy, so just set it here.
  */
 memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
-&vt82c->superio);
+&s->superio);
 }
 
 static void via_class_init(ObjectClass *klass, void *data)
-- 
2.21.3

[PATCH 7/7] vt82c686: Make vt82c686b-pm an abstract base class and add vt8231-pm based on it

The vt82c686b-pm model can be shared between VT82C686B and VT8231. The
only difference between the two is the device id in what we model so
make an abstract via-pm model by renaming appropriately and add types
for vt82c686b-pm and vt8231-pm based on it.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/vt82c686.c | 87 ++-
 include/hw/isa/vt82c686.h |  1 +
 2 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index fc2a1f4430..2e269a2c0f 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -27,9 +27,10 @@
 #include "exec/address-spaces.h"
 #include "trace.h"
 
-OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
+#define TYPE_VIA_PM "via-pm"
+OBJECT_DECLARE_SIMPLE_TYPE(ViaPMState, VIA_PM)
 
-struct VT686PMState {
+struct ViaPMState {
 PCIDevice dev;
 MemoryRegion io;
 ACPIREGS ar;
@@ -37,7 +38,7 @@ struct VT686PMState {
 PMSMBus smb;
 };
 
-static void pm_io_space_update(VT686PMState *s)
+static void pm_io_space_update(ViaPMState *s)
 {
 uint32_t pmbase = pci_get_long(s->dev.config + 0x48) & 0xff80UL;
 
@@ -47,7 +48,7 @@ static void pm_io_space_update(VT686PMState *s)
 memory_region_transaction_commit();
 }
 
-static void smb_io_space_update(VT686PMState *s)
+static void smb_io_space_update(ViaPMState *s)
 {
 uint32_t smbase = pci_get_long(s->dev.config + 0x90) & 0xfff0UL;
 
@@ -59,7 +60,7 @@ static void smb_io_space_update(VT686PMState *s)
 
 static int vmstate_acpi_post_load(void *opaque, int version_id)
 {
-VT686PMState *s = opaque;
+ViaPMState *s = opaque;
 
 pm_io_space_update(s);
 smb_io_space_update(s);
@@ -72,20 +73,20 @@ static const VMStateDescription vmstate_acpi = {
 .minimum_version_id = 1,
 .post_load = vmstate_acpi_post_load,
 .fields = (VMStateField[]) {
-VMSTATE_PCI_DEVICE(dev, VT686PMState),
-VMSTATE_UINT16(ar.pm1.evt.sts, VT686PMState),
-VMSTATE_UINT16(ar.pm1.evt.en, VT686PMState),
-VMSTATE_UINT16(ar.pm1.cnt.cnt, VT686PMState),
-VMSTATE_STRUCT(apm, VT686PMState, 0, vmstate_apm, APMState),
-VMSTATE_TIMER_PTR(ar.tmr.timer, VT686PMState),
-VMSTATE_INT64(ar.tmr.overflow_time, VT686PMState),
+VMSTATE_PCI_DEVICE(dev, ViaPMState),
+VMSTATE_UINT16(ar.pm1.evt.sts, ViaPMState),
+VMSTATE_UINT16(ar.pm1.evt.en, ViaPMState),
+VMSTATE_UINT16(ar.pm1.cnt.cnt, ViaPMState),
+VMSTATE_STRUCT(apm, ViaPMState, 0, vmstate_apm, APMState),
+VMSTATE_TIMER_PTR(ar.tmr.timer, ViaPMState),
+VMSTATE_INT64(ar.tmr.overflow_time, ViaPMState),
 VMSTATE_END_OF_LIST()
 }
 };
 
 static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
 {
-VT686PMState *s = VT82C686B_PM(d);
+ViaPMState *s = VIA_PM(d);
 
 trace_via_pm_write(addr, val, len);
 pci_default_write_config(d, addr, val, len);
@@ -127,7 +128,7 @@ static const MemoryRegionOps pm_io_ops = {
 },
 };
 
-static void pm_update_sci(VT686PMState *s)
+static void pm_update_sci(ViaPMState *s)
 {
 int sci_level, pmsts;
 
@@ -145,13 +146,13 @@ static void pm_update_sci(VT686PMState *s)
 
 static void pm_tmr_timer(ACPIREGS *ar)
 {
-VT686PMState *s = container_of(ar, VT686PMState, ar);
+ViaPMState *s = container_of(ar, ViaPMState, ar);
 pm_update_sci(s);
 }
 
-static void vt82c686b_pm_reset(DeviceState *d)
+static void via_pm_reset(DeviceState *d)
 {
-VT686PMState *s = VT82C686B_PM(d);
+ViaPMState *s = VIA_PM(d);
 
 memset(s->dev.config + PCI_CONFIG_HEADER_SIZE, 0,
PCI_CONFIG_SPACE_SIZE - PCI_CONFIG_HEADER_SIZE);
@@ -164,9 +165,9 @@ static void vt82c686b_pm_reset(DeviceState *d)
 smb_io_space_update(s);
 }
 
-static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
+static void via_pm_realize(PCIDevice *dev, Error **errp)
 {
-VT686PMState *s = VT82C686B_PM(dev);
+ViaPMState *s = VIA_PM(dev);
 
 pci_set_word(dev->config + PCI_STATUS, PCI_STATUS_FAST_BACK |
  PCI_STATUS_DEVSEL_MEDIUM);
@@ -177,8 +178,7 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error 
**errp)
 
 apm_init(dev, &s->apm, NULL, s);
 
-memory_region_init_io(&s->io, OBJECT(dev), &pm_io_ops, s,
-  "vt82c686-pm", 0x100);
+memory_region_init_io(&s->io, OBJECT(dev), &pm_io_ops, s, "via-pm", 0x100);
 memory_region_add_subregion(pci_address_space_io(dev), 0, &s->io);
 memory_region_set_enabled(&s->io, false);
 
@@ -187,34 +187,61 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error 
**errp)
 acpi_pm1_cnt_init(&s->ar, &s->io, false, false, 2);
 }
 
+typedef struct via_pm_init_info {
+uint16_t device_id;
+} ViaPMInitInfo;
+
 static void via_pm_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ViaPMInitInfo *info = data;
 
-k->realize = vt82c686b_pm_realize;
+k->realize = via_pm

[PATCH 6/7] vt82c686: Fix up power management io base and config

Similar to the SMBus io registers there is a power management io range
that's set via similar base address reg and enable bit. Some handling
of this was already there but with several problems: using the wrong
registers and bits, wrong size range, not acually updating mapping and
handling reset correctly or emulating any of the acrual io registers.
Some of these are fixed up here.

After this patch we use the correct base address register, enable bit
and region size and allow guests to map/unmap this region and
correctly reset all registers to default values on reset but we still
don't emulate any of the registers in this range.

Previously just an empty RAM region was mapped on realize, now I've
added empty io range logging access. I think the pm timer should be
hooked up here but not sure guests need it. PMON on fuloong2e sets
base address but does not seem to enable region; the pegasos2 firmware
pokes some regs but continues anyway so don't know if anything would
make use of these facilities. Therefore this is just a clean up of
previous state for now and not intending to fully implement missing
functionality which could be done later if some guests need it.

Signed-off-by: BALATON Zoltan 
---
 hw/isa/trace-events |  2 ++
 hw/isa/vt82c686.c   | 56 -
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/hw/isa/trace-events b/hw/isa/trace-events
index d267d3e652..641d69eedf 100644
--- a/hw/isa/trace-events
+++ b/hw/isa/trace-events
@@ -17,5 +17,7 @@ apm_io_write(uint8_t addr, uint8_t val) "write addr=0x%x 
val=0x%02x"
 # vt82c686.c
 via_isa_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 
0x%x"
 via_pm_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 
0x%x"
+via_pm_io_read(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 
0x%x"
+via_pm_io_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 
0x%x"
 via_superio_read(uint8_t addr, uint8_t val) "addr 0x%x val 0x%x"
 via_superio_write(uint8_t addr, uint32_t val) "addr 0x%x val 0x%x"
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9c4d153022..fc2a1f4430 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -39,14 +39,11 @@ struct VT686PMState {
 
 static void pm_io_space_update(VT686PMState *s)
 {
-uint32_t pm_io_base;
-
-pm_io_base = pci_get_long(s->dev.config + 0x40);
-pm_io_base &= 0xffc0;
+uint32_t pmbase = pci_get_long(s->dev.config + 0x48) & 0xff80UL;
 
 memory_region_transaction_begin();
-memory_region_set_enabled(&s->io, s->dev.config[0x80] & 1);
-memory_region_set_address(&s->io, pm_io_base);
+memory_region_set_address(&s->io, pmbase);
+memory_region_set_enabled(&s->io, s->dev.config[0x41] & BIT(7));
 memory_region_transaction_commit();
 }
 
@@ -92,6 +89,13 @@ static void pm_write_config(PCIDevice *d, uint32_t addr, 
uint32_t val, int len)
 
 trace_via_pm_write(addr, val, len);
 pci_default_write_config(d, addr, val, len);
+if (ranges_overlap(addr, len, 0x48, 4)) {
+uint32_t v = pci_get_long(s->dev.config + 0x48);
+pci_set_long(s->dev.config + 0x48, (v & 0xff80UL) | 1);
+}
+if (range_covers_byte(addr, len, 0x41)) {
+pm_io_space_update(s);
+}
 if (ranges_overlap(addr, len, 0x90, 4)) {
 uint32_t v = pci_get_long(s->dev.config + 0x90);
 pci_set_long(s->dev.config + 0x90, (v & 0xfff0UL) | 1);
@@ -102,6 +106,27 @@ static void pm_write_config(PCIDevice *d, uint32_t addr, 
uint32_t val, int len)
 }
 }
 
+static void pm_io_write(void *op, hwaddr addr, uint64_t data, unsigned size)
+{
+trace_via_pm_io_write(addr, data, size);
+}
+
+static uint64_t pm_io_read(void *op, hwaddr addr, unsigned size)
+{
+trace_via_pm_io_read(addr, 0, size);
+return 0;
+}
+
+static const MemoryRegionOps pm_io_ops = {
+.read = pm_io_read,
+.write = pm_io_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
+};
+
 static void pm_update_sci(VT686PMState *s)
 {
 int sci_level, pmsts;
@@ -128,35 +153,34 @@ static void vt82c686b_pm_reset(DeviceState *d)
 {
 VT686PMState *s = VT82C686B_PM(d);
 
+memset(s->dev.config + PCI_CONFIG_HEADER_SIZE, 0,
+   PCI_CONFIG_SPACE_SIZE - PCI_CONFIG_HEADER_SIZE);
+/* Power Management IO base */
+pci_set_long(s->dev.config + 0x48, 1);
 /* SMBus IO base */
 pci_set_long(s->dev.config + 0x90, 1);
-s->dev.config[0xd2] = 0;
 
+pm_io_space_update(s);
 smb_io_space_update(s);
 }
 
 static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
 {
 VT686PMState *s = VT82C686B_PM(dev);
-uint8_t *pci_conf;
 
-pci_conf = s->dev.config;
-pci_set_word(pci_conf + PCI_COMMAND, 0);
-pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_FAST_BACK |
+pci_set_word(dev->config + PCI_STATUS, PCI_STATUS_FAST_BACK |
  PCI_STATUS_DEVSEL_MEDIUM);
 
-/* 0x4

Re: [PATCH] gdb: riscv: Add target description

2020-12-29 Thread Bin Meng

Hi Sylvain,

On Wed, Dec 30, 2020 at 12:37 AM Sylvain Pelissier
 wrote:
>
> Thank you for your remark here is the new patch:

This should not be put into the commit message.

Previous commit message is missing.

>
> Signed-off-by: Sylvain Pelissier 
> ---
>  target/riscv/cpu.c | 13 +
>  1 file changed, 13 insertions(+)
>

Regards,
Bin

[Bug 1908551] Re: aarch64 SVE emulation breaks strnlen and strrchr