Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

2022-09-08 Thread Bernhard Beschow
Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :
>v5:
>
>* Add patch "Inline vt82c686b_southbridge_init() and remove it" (Zoltan)
>
>* Use machine parameter when creating rtc-time alias (Zoltan)
>
>
>
>Testing done: Same as in v3.
>
>
>
>v4:
>
>* Fix in comment: AC97 Modem -> MC97 Modem (Zoltan)
>
>* Introduce TYPE_VT82C686B_USB_UHCI define (Zoltan)
>
>* Introduce TYPE_VIA_IDE define (for consistency)
>
>
>
>v3:
>
>* Replace pre increment by post increment in for loop (Zoltan)
>
>* Move class defines close to where the class is defined (Zoltan)
>
>
>
>Testing done:
>
>* `make check-avocado`
>
>  Passes for boot_linux_console.py for mips64el_fuloong2e
>
>* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
>ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
>morphos-3.17/boot.img`
>
>  Boots successfully and it is possible to open games and tools.
>
>
>
>v2:
>
>* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)
>
>* Create rtc-time alias in board rather than in south bridge code
>
>* Remove stale comments about PCI functions (Zoltan)
>
>
>
>v1:
>
>This series instantiates all PCI functions of the VT82xx south bridges in the 
>south bridges themselves.
>
>For the IDE function this is especially important since its interrupt routing 
>is configured in the
>
>ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" 
>device. The interrupt
>
>routing is currently hardcoded and changing that is currently not in the scope 
>of this series.
>
>
>
>Testing done:
>
>* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
>ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
>morphos-3.17/boot.img`
>
>  Boots successfully and it is possible to open games and tools.
>
>
>
>* I was unable to test the fuloong2e board even before this series since it 
>seems to be unfinished [1].
>
>  A buildroot-baked kernel [2] booted but doesn't find its root partition, 
> though the issues could be in the buildroot receipt I created.
>
>
>
>[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2
>
>[2] https://github.com/shentok/buildroot/commits/fuloong2e
>

Ping

Zoltan, would you mind giving your Reviewed-by for 'hw/mips/fuloong2e: Inline 
vt82c686b_southbridge_init() and remove it' explicitly? Perhaps I was too eager 
to omit it since I didn't want to put words in your mouth.

What else is missing? Who would do the pull request?

Thanks,
Bernhard
>
>
>Bernhard Beschow (13):
>
>  hw/isa/vt82c686: Resolve chip-specific realize methods
>
>  hw/isa/vt82c686: Resolve unneeded attribute
>
>  hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()
>
>  hw/isa/vt82c686: Reuse errp
>
>  hw/isa/vt82c686: Introduce TYPE_VIA_IDE define
>
>  hw/isa/vt82c686: Instantiate IDE function in host device
>
>  hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define
>
>  hw/isa/vt82c686: Instantiate USB functions in host device
>
>  hw/isa/vt82c686: Instantiate PM function in host device
>
>  hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device
>
>  hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it
>
>  hw/isa/vt82c686: Embed RTCState in host device
>
>  hw/isa/vt82c686: Create rtc-time alias in boards instead
>
>
>
> configs/devices/mips64el-softmmu/default.mak |   1 -
>
> hw/ide/via.c |   2 +-
>
> hw/isa/Kconfig   |   1 +
>
> hw/isa/vt82c686.c| 120 +++
>
> hw/mips/fuloong2e.c  |  39 +++---
>
> hw/ppc/Kconfig   |   1 -
>
> hw/ppc/pegasos2.c|  25 ++--
>
> hw/usb/vt82c686-uhci-pci.c   |   4 +-
>
> include/hw/isa/vt82c686.h|   4 +-
>
> 9 files changed, 126 insertions(+), 71 deletions(-)
>
>
>
>-- >
>2.37.3
>
>
>




Re: [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

2022-09-08 Thread Kevin Wolf
Am 07.09.2022 um 19:25 hat Denis V. Lunev geschrieben:
> ping

I'll get to it (and quite a few other small series on the list that
should be quick to review), but probably only after KVM Forum. So I'll
have to ask you to be patient for a little longer.

Kevin




Re: [PATCH v5 05/13] hw/isa/vt82c686: Introduce TYPE_VIA_IDE define

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

Establishes consistency with other (VIA) devices.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/ide/via.c  | 2 +-
  hw/mips/fuloong2e.c   | 2 +-
  hw/ppc/pegasos2.c | 2 +-
  include/hw/isa/vt82c686.h | 1 +
  4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 82def819c4..e1a429405d 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -230,7 +230,7 @@ static void via_ide_class_init(ObjectClass *klass, void 
*data)
  }
  
  static const TypeInfo via_ide_info = {

-.name  = "via-ide",
+.name  = TYPE_VIA_IDE,
  .parent= TYPE_PCI_IDE,
  .class_init= via_ide_class_init,
  };
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 5ee546f5f6..44225fbe33 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -205,7 +205,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
TYPE_VT82C686B_ISA);
  qdev_connect_gpio_out(DEVICE(dev), 0, intc);
  
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");

+dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), TYPE_VIA_IDE);
  pci_ide_create_devs(dev);
  
  pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..8039775f80 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -166,7 +166,7 @@ static void pegasos2_init(MachineState *machine)
qdev_get_gpio_in_named(pm->mv, "gpp", 31));
  
  /* VT8231 function 1: IDE Controller */

-dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), "via-ide");
+dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), TYPE_VIA_IDE);
  pci_ide_create_devs(dev);
  
  /* VT8231 function 2-3: USB Ports */

diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 56ac141be3..87aca3e5bb 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -8,6 +8,7 @@
  #define TYPE_VT8231_ISA "vt8231-isa"
  #define TYPE_VT8231_PM "vt8231-pm"
  #define TYPE_VIA_AC97 "via-ac97"
+#define TYPE_VIA_IDE "via-ide"
  #define TYPE_VIA_MC97 "via-mc97"
  
  void via_isa_set_irq(PCIDevice *d, int n, int level);




Re: [PATCH v5 06/13] hw/isa/vt82c686: Instantiate IDE function in host device

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

The IDE function is closely tied to the ISA function (e.g. the IDE
interrupt routing happens there), so it makes sense that the IDE
function is instantiated within the south bridge itself.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  configs/devices/mips64el-softmmu/default.mak |  1 -
  hw/isa/Kconfig   |  1 +
  hw/isa/vt82c686.c| 17 +
  hw/mips/fuloong2e.c  |  8 
  hw/ppc/Kconfig   |  1 -
  hw/ppc/pegasos2.c|  9 -
  6 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/configs/devices/mips64el-softmmu/default.mak 
b/configs/devices/mips64el-softmmu/default.mak
index c610749ac1..d5188f7ea5 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -1,7 +1,6 @@
  # Default configuration for mips64el-softmmu
  
  include ../mips-softmmu/common.mak

-CONFIG_IDE_VIA=y
  CONFIG_FULOONG=y
  CONFIG_LOONGSON3V=y
  CONFIG_ATI_VGA=y
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index d42143a991..20de7e9294 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -53,6 +53,7 @@ config VT82C686
  select I8254
  select I8257
  select I8259
+select IDE_VIA
  select MC146818RTC
  select PARALLEL
  
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c

index 37e37b3855..63c1e3b8ce 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -17,6 +17,7 @@
  #include "hw/isa/vt82c686.h"
  #include "hw/pci/pci.h"
  #include "hw/qdev-properties.h"
+#include "hw/ide/pci.h"
  #include "hw/isa/isa.h"
  #include "hw/isa/superio.h"
  #include "hw/intc/i8259.h"
@@ -544,6 +545,7 @@ struct ViaISAState {
  qemu_irq cpu_intr;
  qemu_irq *isa_irqs;
  ViaSuperIOState via_sio;
+PCIIDEState ide;
  };
  
  static const VMStateDescription vmstate_via = {

@@ -556,10 +558,18 @@ static const VMStateDescription vmstate_via = {
  }
  };
  
+static void via_isa_init(Object *obj)

+{
+ViaISAState *s = VIA_ISA(obj);
+
+object_initialize_child(obj, "ide", &s->ide, TYPE_VIA_IDE);
+}
+
  static const TypeInfo via_isa_info = {
  .name  = TYPE_VIA_ISA,
  .parent= TYPE_PCI_DEVICE,
  .instance_size = sizeof(ViaISAState),
+.instance_init = via_isa_init,
  .abstract  = true,
  .interfaces= (InterfaceInfo[]) {
  { INTERFACE_CONVENTIONAL_PCI_DEVICE },
@@ -583,6 +593,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  {
  ViaISAState *s = VIA_ISA(d);
  DeviceState *dev = DEVICE(d);
+PCIBus *pci_bus = pci_get_bus(d);
  qemu_irq *isa_irq;
  ISABus *isa_bus;
  int i;
@@ -612,6 +623,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->via_sio), BUS(isa_bus), errp)) {
  return;
  }
+
+/* Function 1: IDE */
+qdev_prop_set_int32(DEVICE(&s->ide), "addr", d->devfn + 1);
+if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+return;
+}
  }
  
  /* TYPE_VT82C686B_ISA */

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 44225fbe33..32605901e7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -199,13 +199,13 @@ static void main_cpu_reset(void *opaque)
  static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq 
intc,
 I2CBus **i2c_bus)
  {
-PCIDevice *dev;
+PCIDevice *dev, *via;
  
-dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,

+via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
TYPE_VT82C686B_ISA);
-qdev_connect_gpio_out(DEVICE(dev), 0, intc);
+qdev_connect_gpio_out(DEVICE(via), 0, intc);
  
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), TYPE_VIA_IDE);

+dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
  pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 400511c6b7..18565e966b 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -74,7 +74,6 @@ config PEGASOS2
  bool
  select MV64361
  select VT82C686
-select IDE_VIA
  select SMBUS_EEPROM
  select VOF
  # This should come with VT82C686
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 8039775f80..8bc528a560 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -102,7 +102,7 @@ static void pegasos2_init(MachineState *machine)
  CPUPPCState *env;
  MemoryRegion *rom = g_new(MemoryRegion, 1);
  PCIBus *pci_bus;
-PCIDevice *dev;
+PCIDevice *dev, *via;
  I2CBus *i2c_bus;
  const char *fwname = machine->firmware ?: PROM_FILENAME;
  char *filename;
@@ -160,13 +160,12 @@ static void pegasos2_init(MachineState

Re: [PATCH v5 08/13] hw/isa/vt82c686: Instantiate USB functions in host device

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

The USB functions can be enabled/disabled through the ISA function. Also
its interrupt routing can be influenced there.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c   | 12 
  hw/mips/fuloong2e.c |  3 ---
  hw/ppc/pegasos2.c   |  4 
  3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 63c1e3b8ce..f05fd9948a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -23,6 +23,7 @@
  #include "hw/intc/i8259.h"
  #include "hw/irq.h"
  #include "hw/dma/i8257.h"
+#include "hw/usb/hcd-uhci.h"
  #include "hw/timer/i8254.h"
  #include "hw/rtc/mc146818rtc.h"
  #include "migration/vmstate.h"
@@ -546,6 +547,7 @@ struct ViaISAState {
  qemu_irq *isa_irqs;
  ViaSuperIOState via_sio;
  PCIIDEState ide;
+UHCIState uhci[2];
  };
  
  static const VMStateDescription vmstate_via = {

@@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
  ViaISAState *s = VIA_ISA(obj);
  
  object_initialize_child(obj, "ide", &s->ide, TYPE_VIA_IDE);

+object_initialize_child(obj, "uhci1", &s->uhci[0], 
TYPE_VT82C686B_USB_UHCI);
+object_initialize_child(obj, "uhci2", &s->uhci[1], 
TYPE_VT82C686B_USB_UHCI);
  }
  
  static const TypeInfo via_isa_info = {

@@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
  return;
  }
+
+/* Functions 2-3: USB Ports */
+for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
+qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
+if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
+return;
+}
+}
  }
  
  /* TYPE_VT82C686B_ISA */

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 6b7370f2aa..dc92223b76 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), TYPE_VT82C686B_USB_UHCI);

-pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), TYPE_VT82C686B_USB_UHCI);
-
  dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
  
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c

index 70776558c9..85cca6f7a6 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-/* VT8231 function 2-3: USB Ports */

-pci_create_simple(pci_bus, PCI_DEVFN(12, 2), TYPE_VT82C686B_USB_UHCI);
-pci_create_simple(pci_bus, PCI_DEVFN(12, 3), TYPE_VT82C686B_USB_UHCI);
-
  /* VT8231 function 4: Power Management Controller */
  dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
  i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));




Re: [PATCH v5 07/13] hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

Suggested-by: BALATON Zoltan 
Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/mips/fuloong2e.c| 4 ++--
  hw/ppc/pegasos2.c  | 4 ++--
  hw/usb/vt82c686-uhci-pci.c | 4 ++--
  include/hw/isa/vt82c686.h  | 1 +
  4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 32605901e7..6b7370f2aa 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,8 +208,8 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");

-pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
+pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), TYPE_VT82C686B_USB_UHCI);
+pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), TYPE_VT82C686B_USB_UHCI);
  
  dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);

  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 8bc528a560..70776558c9 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -169,8 +169,8 @@ static void pegasos2_init(MachineState *machine)
  pci_ide_create_devs(dev);
  
  /* VT8231 function 2-3: USB Ports */

-pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
-pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
+pci_create_simple(pci_bus, PCI_DEVFN(12, 2), TYPE_VT82C686B_USB_UHCI);
+pci_create_simple(pci_bus, PCI_DEVFN(12, 3), TYPE_VT82C686B_USB_UHCI);
  
  /* VT8231 function 4: Power Management Controller */

  dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
index 0bf2b72ff0..46a901f56f 100644
--- a/hw/usb/vt82c686-uhci-pci.c
+++ b/hw/usb/vt82c686-uhci-pci.c
@@ -31,7 +31,7 @@ static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error 
**errp)
  
  static UHCIInfo uhci_info[] = {

  {
-.name  = "vt82c686b-usb-uhci",
+.name  = TYPE_VT82C686B_USB_UHCI,
  .vendor_id = PCI_VENDOR_ID_VIA,
  .device_id = PCI_DEVICE_ID_VIA_UHCI,
  .revision  = 0x01,
@@ -45,7 +45,7 @@ static UHCIInfo uhci_info[] = {
  
  static const TypeInfo vt82c686b_usb_uhci_type_info = {

  .parent = TYPE_UHCI,
-.name   = "vt82c686b-usb-uhci",
+.name   = TYPE_VT82C686B_USB_UHCI,
  .class_init = uhci_data_class_init,
  .class_data = uhci_info,
  };
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 87aca3e5bb..e6f6dd4d43 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -5,6 +5,7 @@
  
  #define TYPE_VT82C686B_ISA "vt82c686b-isa"

  #define TYPE_VT82C686B_PM "vt82c686b-pm"
+#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
  #define TYPE_VT8231_ISA "vt8231-isa"
  #define TYPE_VT8231_PM "vt8231-pm"
  #define TYPE_VIA_AC97 "via-ac97"




Re: [PATCH v5 09/13] hw/isa/vt82c686: Instantiate PM function in host device

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

The PM controller has activity bits which monitor activity of other
built-in devices in the host device.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c | 13 +
  hw/mips/fuloong2e.c   |  2 +-
  hw/ppc/pegasos2.c |  3 +--
  include/hw/isa/vt82c686.h |  2 --
  4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index f05fd9948a..d048607079 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -250,6 +250,8 @@ static const ViaPMInitInfo vt82c686b_pm_init_info = {
  .device_id = PCI_DEVICE_ID_VIA_82C686B_PM,
  };
  
+#define TYPE_VT82C686B_PM "vt82c686b-pm"

+
  static const TypeInfo vt82c686b_pm_info = {
  .name  = TYPE_VT82C686B_PM,
  .parent= TYPE_VIA_PM,
@@ -261,6 +263,8 @@ static const ViaPMInitInfo vt8231_pm_init_info = {
  .device_id = PCI_DEVICE_ID_VIA_8231_PM,
  };
  
+#define TYPE_VT8231_PM "vt8231-pm"

+
  static const TypeInfo vt8231_pm_info = {
  .name  = TYPE_VT8231_PM,
  .parent= TYPE_VIA_PM,
@@ -548,6 +552,7 @@ struct ViaISAState {
  ViaSuperIOState via_sio;
  PCIIDEState ide;
  UHCIState uhci[2];
+ViaPMState pm;
  };
  
  static const VMStateDescription vmstate_via = {

@@ -641,6 +646,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  return;
  }
  }
+
+/* Function 4: Power Management */
+qdev_prop_set_int32(DEVICE(&s->pm), "addr", d->devfn + 4);
+if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+return;
+}
  }
  
  /* TYPE_VT82C686B_ISA */

@@ -683,6 +694,7 @@ static void vt82c686b_init(Object *obj)
  ViaISAState *s = VIA_ISA(obj);
  
  object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT82C686B_SUPERIO);

+object_initialize_child(obj, "pm", &s->pm, TYPE_VT82C686B_PM);
  }
  
  static void vt82c686b_class_init(ObjectClass *klass, void *data)

@@ -746,6 +758,7 @@ static void vt8231_init(Object *obj)
  ViaISAState *s = VIA_ISA(obj);
  
  object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT8231_SUPERIO);

+object_initialize_child(obj, "pm", &s->pm, TYPE_VT8231_PM);
  }
  
  static void vt8231_class_init(ObjectClass *klass, void *data)

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index dc92223b76..377108d313 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,7 +208,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int 
slot, qemu_irq intc,
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);

+dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
  
  /* Audio support */

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 85cca6f7a6..e32944ee2b 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,8 +168,7 @@ static void pegasos2_init(MachineState *machine)
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
  pci_ide_create_devs(dev);
  
-/* VT8231 function 4: Power Management Controller */

-dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
+dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
  i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
  spd_data = spd_data_generate(DDR, machine->ram_size);
  smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index e6f6dd4d43..eaa07881c5 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -4,10 +4,8 @@
  #include "hw/pci/pci.h"
  
  #define TYPE_VT82C686B_ISA "vt82c686b-isa"

-#define TYPE_VT82C686B_PM "vt82c686b-pm"
  #define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
  #define TYPE_VT8231_ISA "vt8231-isa"
-#define TYPE_VT8231_PM "vt8231-pm"
  #define TYPE_VIA_AC97 "via-ac97"
  #define TYPE_VIA_IDE "via-ide"
  #define TYPE_VIA_MC97 "via-mc97"




Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

2022-09-08 Thread BALATON Zoltan

On Thu, 8 Sep 2022, Bernhard Beschow wrote:

Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :

v5:

* Add patch "Inline vt82c686b_southbridge_init() and remove it" (Zoltan)

* Use machine parameter when creating rtc-time alias (Zoltan)



Testing done: Same as in v3.



v4:

* Fix in comment: AC97 Modem -> MC97 Modem (Zoltan)

* Introduce TYPE_VT82C686B_USB_UHCI define (Zoltan)

* Introduce TYPE_VIA_IDE define (for consistency)



v3:

* Replace pre increment by post increment in for loop (Zoltan)

* Move class defines close to where the class is defined (Zoltan)



Testing done:

* `make check-avocado`

 Passes for boot_linux_console.py for mips64el_fuloong2e

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

 Boots successfully and it is possible to open games and tools.



v2:

* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)

* Create rtc-time alias in board rather than in south bridge code

* Remove stale comments about PCI functions (Zoltan)



v1:

This series instantiates all PCI functions of the VT82xx south bridges in the 
south bridges themselves.

For the IDE function this is especially important since its interrupt routing 
is configured in the

ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" 
device. The interrupt

routing is currently hardcoded and changing that is currently not in the scope 
of this series.



Testing done:

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

 Boots successfully and it is possible to open games and tools.



* I was unable to test the fuloong2e board even before this series since it 
seems to be unfinished [1].

 A buildroot-baked kernel [2] booted but doesn't find its root partition, 
though the issues could be in the buildroot receipt I created.



[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2

[2] https://github.com/shentok/buildroot/commits/fuloong2e



Ping

Zoltan, would you mind giving your Reviewed-by for 'hw/mips/fuloong2e: 
Inline vt82c686b_southbridge_init() and remove it' explicitly? Perhaps I 
was too eager to omit it since I didn't want to put words in your mouth.


You said in your follow up message that all except this patch has my R-b 
which is correct. This one already has Suggested-by from me so I agree 
with it with or without an explicit Reviewed-by.



What else is missing? Who would do the pull request?


It was Philippe before who merged these maybe needs his attention or give 
some Ack to go via smoe other tree? My mails don't seem to reach him 
though due to bouncing as spam so not sure he sees this.


Regards,
BALATON Zoltan


Thanks,
Bernhard



Bernhard Beschow (13):

 hw/isa/vt82c686: Resolve chip-specific realize methods

 hw/isa/vt82c686: Resolve unneeded attribute

 hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()

 hw/isa/vt82c686: Reuse errp

 hw/isa/vt82c686: Introduce TYPE_VIA_IDE define

 hw/isa/vt82c686: Instantiate IDE function in host device

 hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define

 hw/isa/vt82c686: Instantiate USB functions in host device

 hw/isa/vt82c686: Instantiate PM function in host device

 hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

 hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it

 hw/isa/vt82c686: Embed RTCState in host device

 hw/isa/vt82c686: Create rtc-time alias in boards instead



configs/devices/mips64el-softmmu/default.mak |   1 -

hw/ide/via.c |   2 +-

hw/isa/Kconfig   |   1 +

hw/isa/vt82c686.c| 120 +++

hw/mips/fuloong2e.c  |  39 +++---

hw/ppc/Kconfig   |   1 -

hw/ppc/pegasos2.c|  25 ++--

hw/usb/vt82c686-uhci-pci.c   |   4 +-

include/hw/isa/vt82c686.h|   4 +-

9 files changed, 126 insertions(+), 71 deletions(-)



-- >
2.37.3











Re: [PATCH v5 10/13] hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

The AC97 function's wakeup status is wired to the PM function and both
the AC97 and MC97 interrupt routing is determined by the ISA function.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c   | 16 
  hw/mips/fuloong2e.c |  4 
  hw/ppc/pegasos2.c   |  5 -
  3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d048607079..91686e9570 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -553,6 +553,8 @@ struct ViaISAState {
  PCIIDEState ide;
  UHCIState uhci[2];
  ViaPMState pm;
+PCIDevice ac97;
+PCIDevice mc97;
  };
  
  static const VMStateDescription vmstate_via = {

@@ -572,6 +574,8 @@ static void via_isa_init(Object *obj)
  object_initialize_child(obj, "ide", &s->ide, TYPE_VIA_IDE);
  object_initialize_child(obj, "uhci1", &s->uhci[0], 
TYPE_VT82C686B_USB_UHCI);
  object_initialize_child(obj, "uhci2", &s->uhci[1], 
TYPE_VT82C686B_USB_UHCI);
+object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
+object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
  }
  
  static const TypeInfo via_isa_info = {

@@ -652,6 +656,18 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
  return;
  }
+
+/* Function 5: AC97 Audio */
+qdev_prop_set_int32(DEVICE(&s->ac97), "addr", d->devfn + 5);
+if (!qdev_realize(DEVICE(&s->ac97), BUS(pci_bus), errp)) {
+return;
+}
+
+/* Function 6: MC97 Modem */
+qdev_prop_set_int32(DEVICE(&s->mc97), "addr", d->devfn + 6);
+if (!qdev_realize(DEVICE(&s->mc97), BUS(pci_bus), errp)) {
+return;
+}
  }
  
  /* TYPE_VT82C686B_ISA */

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 377108d313..2d8723ab74 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -210,10 +210,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, 
int slot, qemu_irq intc,
  
  dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));

  *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
-
-/* Audio support */
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
-pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
  }
  
  /* Network support */

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index e32944ee2b..09fdb7557f 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -159,7 +159,6 @@ static void pegasos2_init(MachineState *machine)
  pci_bus = mv64361_get_pci_bus(pm->mv, 1);
  
  /* VIA VT8231 South Bridge (multifunction PCI device) */

-/* VT8231 function 0: PCI-to-ISA Bridge */
  via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
TYPE_VT8231_ISA);
  qdev_connect_gpio_out(DEVICE(via), 0,
@@ -173,10 +172,6 @@ static void pegasos2_init(MachineState *machine)
  spd_data = spd_data_generate(DDR, machine->ram_size);
  smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
  
-/* VT8231 function 5-6: AC97 Audio & Modem */

-pci_create_simple(pci_bus, PCI_DEVFN(12, 5), TYPE_VIA_AC97);
-pci_create_simple(pci_bus, PCI_DEVFN(12, 6), TYPE_VIA_MC97);
-
  /* other PC hardware */
  pci_vga_init(pci_bus);
  




Re: [PATCH v5 13/13] hw/isa/vt82c686: Create rtc-time alias in boards instead

2022-09-08 Thread Daniel Henrique Barboza




On 9/1/22 08:41, Bernhard Beschow wrote:

According to good QOM practice, an object should only deal with objects
of its own sub tree. Having devices create an alias on the machine
object doesn't respect this good practice. To resolve this, create the
alias in the machine's code.

Signed-off-by: Bernhard Beschow 
---


Acked-by: Daniel Henrique Barboza 


  hw/isa/vt82c686.c   | 2 --
  hw/mips/fuloong2e.c | 4 
  hw/ppc/pegasos2.c   | 4 
  3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 48cd4d0036..3f9bd0c04d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -632,8 +632,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
  if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
  return;
  }
-object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(&s->rtc),
-  "date");
  isa_connect_gpio_out(ISA_DEVICE(&s->rtc), 0, s->rtc.isairq);
  
  for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {

diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 3c46215616..b478483706 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -295,6 +295,10 @@ static void mips_fuloong2e_init(MachineState *machine)
  pci_dev = pci_create_simple_multifunction(pci_bus,
PCI_DEVFN(FULOONG2E_VIA_SLOT, 
0),
true, TYPE_VT82C686B_ISA);
+object_property_add_alias(OBJECT(machine), "rtc-time",
+  object_resolve_path_component(OBJECT(pci_dev),
+"rtc"),
+  "date");
  qdev_connect_gpio_out(DEVICE(pci_dev), 0, env->irq[5]);
  
  dev = DEVICE(object_resolve_path_component(OBJECT(pci_dev), "ide"));

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 09fdb7557f..49b753c7cc 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -161,6 +161,10 @@ static void pegasos2_init(MachineState *machine)
  /* VIA VT8231 South Bridge (multifunction PCI device) */
  via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
TYPE_VT8231_ISA);
+object_property_add_alias(OBJECT(machine), "rtc-time",
+  object_resolve_path_component(OBJECT(via),
+"rtc"),
+  "date");
  qdev_connect_gpio_out(DEVICE(via), 0,
qdev_get_gpio_in_named(pm->mv, "gpp", 31));
  




Re: [PATCH v5 00/13] Instantiate VT82xx functions in host device

2022-09-08 Thread Daniel Henrique Barboza




On 9/8/22 05:34, Bernhard Beschow wrote:

Am 1. September 2022 11:41:14 UTC schrieb Bernhard Beschow :

v5:

* Add patch "Inline vt82c686b_southbridge_init() and remove it" (Zoltan)

* Use machine parameter when creating rtc-time alias (Zoltan)



Testing done: Same as in v3.



v4:

* Fix in comment: AC97 Modem -> MC97 Modem (Zoltan)

* Introduce TYPE_VT82C686B_USB_UHCI define (Zoltan)

* Introduce TYPE_VIA_IDE define (for consistency)



v3:

* Replace pre increment by post increment in for loop (Zoltan)

* Move class defines close to where the class is defined (Zoltan)



Testing done:

* `make check-avocado`

  Passes for boot_linux_console.py for mips64el_fuloong2e

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

  Boots successfully and it is possible to open games and tools.



v2:

* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)

* Create rtc-time alias in board rather than in south bridge code

* Remove stale comments about PCI functions (Zoltan)



v1:

This series instantiates all PCI functions of the VT82xx south bridges in the 
south bridges themselves.

For the IDE function this is especially important since its interrupt routing 
is configured in the

ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" 
device. The interrupt

routing is currently hardcoded and changing that is currently not in the scope 
of this series.



Testing done:

* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device 
ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel 
morphos-3.17/boot.img`

  Boots successfully and it is possible to open games and tools.



* I was unable to test the fuloong2e board even before this series since it 
seems to be unfinished [1].

  A buildroot-baked kernel [2] booted but doesn't find its root partition, 
though the issues could be in the buildroot receipt I created.



[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2

[2] https://github.com/shentok/buildroot/commits/fuloong2e



Ping

Zoltan, would you mind giving your Reviewed-by for 'hw/mips/fuloong2e: Inline 
vt82c686b_southbridge_init() and remove it' explicitly? Perhaps I was too eager 
to omit it since I didn't want to put words in your mouth.

What else is missing? Who would do the pull request?



The bulk of the changes were done in hw/isa/vt82c686.c and hw/mips/fuloong2e.c.
The Fuloong 2E maintainers are already CCed, so I believe they're already
aware of this series.

I did my test round with the PowerPC test suit with this series and it didn't
break anything, so I acked all patches that changed hw/ppc/pegasos2.c. Feel
free to push those changes in the Fuloong 2E pull request.


Thanks,


Daniel




Thanks,
Bernhard



Bernhard Beschow (13):

  hw/isa/vt82c686: Resolve chip-specific realize methods

  hw/isa/vt82c686: Resolve unneeded attribute

  hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()

  hw/isa/vt82c686: Reuse errp

  hw/isa/vt82c686: Introduce TYPE_VIA_IDE define

  hw/isa/vt82c686: Instantiate IDE function in host device

  hw/isa/vt82c686: Introduce TYPE_VT82C686B_USB_UHCI define

  hw/isa/vt82c686: Instantiate USB functions in host device

  hw/isa/vt82c686: Instantiate PM function in host device

  hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

  hw/mips/fuloong2e: Inline vt82c686b_southbridge_init() and remove it

  hw/isa/vt82c686: Embed RTCState in host device

  hw/isa/vt82c686: Create rtc-time alias in boards instead



configs/devices/mips64el-softmmu/default.mak |   1 -

hw/ide/via.c |   2 +-

hw/isa/Kconfig   |   1 +

hw/isa/vt82c686.c| 120 +++

hw/mips/fuloong2e.c  |  39 +++---

hw/ppc/Kconfig   |   1 -

hw/ppc/pegasos2.c|  25 ++--

hw/usb/vt82c686-uhci-pci.c   |   4 +-

include/hw/isa/vt82c686.h|   4 +-

9 files changed, 126 insertions(+), 71 deletions(-)



-- >
2.37.3










[PATCH v3] 9pfs: use GHashTable for fid table

2022-09-08 Thread Linus Heckemann
The previous implementation would iterate over the fid table for
lookup operations, resulting in an operation with O(n) complexity on
the number of open files and poor cache locality -- for every open,
stat, read, write, etc operation.

This change uses a hashtable for this instead, significantly improving
the performance of the 9p filesystem. The runtime of NixOS's simple
installer test, which copies ~122k files totalling ~1.8GiB from 9p,
decreased by a factor of about 10.

Signed-off-by: Linus Heckemann 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Greg Kurz 
---

Greg Kurz writes:
> The comment above should be adapted to the new situation : no need

I've removed it completely, since the logic is simple enough that only
the shortened comment below remains necessary.

> With the new logic, this should just be:

now is :)

> g_hash_table_steal_extended() [1] actually allows to do just that.

g_hash_table_steal_extended unfortunately isn't available since it was
introduced in glib 2.58 and we're maintaining compatibility to 2.56.

> You could just call g_hash_table_iter_remove(&iter) here

Applied this suggestion, thanks!


> Well... finding at least one clunked fid state in this table is
> definitely a bug so I'll keep the BUG_ON() anyway.

Christian Schoenebeck writes:
> Yeah, I think you are right, it would feel odd. Just drop BUG_ON() for
> now.

I still prefer dropping it, but if we were to keep it I think it should
be in v9fs_reclaim_fd where we iterate and can thus check the whole
table.


Greg Kurz and Philippe Mathieu-Daudé write:
> [patch versioning]

Whoops. I used -v2 on git send-email, which just ignored the option,
rather than git format-patch, by accident. This one _should_ now be v3!


 hw/9pfs/9p.c | 140 +--
 hw/9pfs/9p.h |   2 +-
 2 files changed, 70 insertions(+), 72 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index aebadeaa03..98a475e560 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -282,33 +282,31 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, 
int32_t fid)
 V9fsFidState *f;
 V9fsState *s = pdu->s;
 
-QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
-BUG_ON(f->clunked);
-if (f->fid == fid) {
-/*
- * Update the fid ref upfront so that
- * we don't get reclaimed when we yield
- * in open later.
- */
-f->ref++;
-/*
- * check whether we need to reopen the
- * file. We might have closed the fd
- * while trying to free up some file
- * descriptors.
- */
-err = v9fs_reopen_fid(pdu, f);
-if (err < 0) {
-f->ref--;
-return NULL;
-}
-/*
- * Mark the fid as referenced so that the LRU
- * reclaim won't close the file descriptor
- */
-f->flags |= FID_REFERENCED;
-return f;
+f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
+if (f) {
+/*
+ * Update the fid ref upfront so that
+ * we don't get reclaimed when we yield
+ * in open later.
+ */
+f->ref++;
+/*
+ * check whether we need to reopen the
+ * file. We might have closed the fd
+ * while trying to free up some file
+ * descriptors.
+ */
+err = v9fs_reopen_fid(pdu, f);
+if (err < 0) {
+f->ref--;
+return NULL;
 }
+/*
+ * Mark the fid as referenced so that the LRU
+ * reclaim won't close the file descriptor
+ */
+f->flags |= FID_REFERENCED;
+return f;
 }
 return NULL;
 }
@@ -317,12 +315,9 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
 {
 V9fsFidState *f;
 
-QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
+if (g_hash_table_contains(s->fids, GINT_TO_POINTER(fid))) {
 /* If fid is already there return NULL */
-BUG_ON(f->clunked);
-if (f->fid == fid) {
-return NULL;
-}
+return NULL;
 }
 f = g_new0(V9fsFidState, 1);
 f->fid = fid;
@@ -333,7 +328,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
  * reclaim won't close the file descriptor
  */
 f->flags |= FID_REFERENCED;
-QSIMPLEQ_INSERT_TAIL(&s->fid_list, f, next);
+g_hash_table_insert(s->fids, GINT_TO_POINTER(fid), f);
 
 v9fs_readdir_init(s->proto_version, &f->fs.dir);
 v9fs_readdir_init(s->proto_version, &f->fs_reclaim.dir);
@@ -424,12 +419,11 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
 {
 V9fsFidState *fidp;
 
-QSIMPLEQ_FOREACH(fidp, &s->fid_list, next) {
-if (fidp->fid == fid) {
-QSIMPLEQ_REMOVE(&s->fid_list, fidp, V9fsFidState, next);
-fidp->clunked = true;
-return fidp;
-}
+fidp = g_hash_table_lookup(s->fids,

Re: [PATCH v3] 9pfs: use GHashTable for fid table

2022-09-08 Thread Greg Kurz
On Thu,  8 Sep 2022 13:23:53 +0200
Linus Heckemann  wrote:

> The previous implementation would iterate over the fid table for
> lookup operations, resulting in an operation with O(n) complexity on
> the number of open files and poor cache locality -- for every open,
> stat, read, write, etc operation.
> 
> This change uses a hashtable for this instead, significantly improving
> the performance of the 9p filesystem. The runtime of NixOS's simple
> installer test, which copies ~122k files totalling ~1.8GiB from 9p,
> decreased by a factor of about 10.
> 
> Signed-off-by: Linus Heckemann 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Greg Kurz 
> ---
> 
> Greg Kurz writes:
> > The comment above should be adapted to the new situation : no need
> 
> I've removed it completely, since the logic is simple enough that only
> the shortened comment below remains necessary.
> 
> > With the new logic, this should just be:
> 
> now is :)
> 
> > g_hash_table_steal_extended() [1] actually allows to do just that.
> 
> g_hash_table_steal_extended unfortunately isn't available since it was
> introduced in glib 2.58 and we're maintaining compatibility to 2.56.
> 

Ha... this could be addressed through conditional compilation, e.g.:

static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
{
V9fsFidState *fidp;

#if GLIB_CHECK_VERSION(2,56,0)
if (!g_hash_table_steal_extended(s->fids, GINT_TO_POINTER(fid),
 NULL, (gpointer *) &fidp)) {
return NULL;
}
#else
fidp = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
if (fidp) {
g_hash_table_remove(s->fids, GINT_TO_POINTER(fid));
} else {
return NULL;
}
#endif

fidp->clunked = true;
return fidp;
}

or simply leave a TODO comment so that we don't forget.


> > You could just call g_hash_table_iter_remove(&iter) here
> 
> Applied this suggestion, thanks!
> 
> 
> > Well... finding at least one clunked fid state in this table is
> > definitely a bug so I'll keep the BUG_ON() anyway.
> 
> Christian Schoenebeck writes:
> > Yeah, I think you are right, it would feel odd. Just drop BUG_ON() for
> > now.
> 
> I still prefer dropping it, but if we were to keep it I think it should
> be in v9fs_reclaim_fd where we iterate and can thus check the whole
> table.
> 

IMO the relevant aspect isn't really about checking the whole table, but
rather not to get a clunked fid out of this table and pass it over.

> 
> Greg Kurz and Philippe Mathieu-Daudé write:
> > [patch versioning]
> 
> Whoops. I used -v2 on git send-email, which just ignored the option,
> rather than git format-patch, by accident. This one _should_ now be v3!
> 
> 

v3 it is and LGTM ! No big deal with the BUG_ON(), given the improvement.

My R-b stands. Thanks Linus !

>  hw/9pfs/9p.c | 140 +--
>  hw/9pfs/9p.h |   2 +-
>  2 files changed, 70 insertions(+), 72 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index aebadeaa03..98a475e560 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -282,33 +282,31 @@ static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, 
> int32_t fid)
>  V9fsFidState *f;
>  V9fsState *s = pdu->s;
>  
> -QSIMPLEQ_FOREACH(f, &s->fid_list, next) {
> -BUG_ON(f->clunked);
> -if (f->fid == fid) {
> -/*
> - * Update the fid ref upfront so that
> - * we don't get reclaimed when we yield
> - * in open later.
> - */
> -f->ref++;
> -/*
> - * check whether we need to reopen the
> - * file. We might have closed the fd
> - * while trying to free up some file
> - * descriptors.
> - */
> -err = v9fs_reopen_fid(pdu, f);
> -if (err < 0) {
> -f->ref--;
> -return NULL;
> -}
> -/*
> - * Mark the fid as referenced so that the LRU
> - * reclaim won't close the file descriptor
> - */
> -f->flags |= FID_REFERENCED;
> -return f;
> +f = g_hash_table_lookup(s->fids, GINT_TO_POINTER(fid));
> +if (f) {
> +/*
> + * Update the fid ref upfront so that
> + * we don't get reclaimed when we yield
> + * in open later.
> + */
> +f->ref++;
> +/*
> + * check whether we need to reopen the
> + * file. We might have closed the fd
> + * while trying to free up some file
> + * descriptors.
> + */
> +err = v9fs_reopen_fid(pdu, f);
> +if (err < 0) {
> +f->ref--;
> +return NULL;
>  }
> +/*
> + * Mark the fid as referenced so that the LRU
> + * reclaim won't close the file descriptor
> + */
> +f->flags |= FID_REFERENCED;
> +return f;
>  }
>  return NULL;
>  }
> @@ -317,12 +315,9 @

[PATCH] qemu-img: Wean documentation and help output off '?' for help

2022-09-08 Thread Markus Armbruster
'?' for help is deprecated since commit c8057f951d "Support 'help' as
a synonym for '?' in command line options", v1.2.0.  We neglected to
update output of qemu-img --help and the manual.  Do that now.

Signed-off-by: Markus Armbruster 
---
 docs/tools/qemu-img.rst | 2 +-
 qemu-img.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 85a6e05b35..15aeddc6d8 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -57,7 +57,7 @@ cases. See below for a description of the supported disk 
formats.
 *OUTPUT_FMT* is the destination format.
 
 *OPTIONS* is a comma separated list of format specific options in a
-name=value format. Use ``-o ?`` for an overview of the options supported
+name=value format. Use ``-o help`` for an overview of the options supported
 by the used format or see the format descriptions below for details.
 
 *SNAPSHOT_PARAM* is param used for internal snapshot, format is
diff --git a/qemu-img.c b/qemu-img.c
index 7d4b33b3da..cab9776f42 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -164,8 +164,8 @@ void help(void)
"  'output_filename' is the destination disk image filename\n"
"  'output_fmt' is the destination format\n"
"  'options' is a comma separated list of format specific options 
in a\n"
-   "name=value format. Use -o ? for an overview of the options 
supported by the\n"
-   "used format\n"
+   "name=value format. Use -o help for an overview of the options 
supported by\n"
+   "the used format\n"
"  'snapshot_param' is param used for internal snapshot, format\n"
"is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
"'[ID_OR_NAME]'\n"
-- 
2.37.2




[PATCH 0/7] nsis: gitlab-ci: Improve QEMU Windows installer packaging

2022-09-08 Thread Bin Meng
At present packaging the required DLLs of QEMU executables is a
manual process, and error prone.

Improve scripts/nsis.py by adding a logic to automatically package
required DLLs of QEMU executables.

'make installer' is tested in the cross-build on Linux in CI, but
not in the Windows native build. Update CI to test the installer
generation on Windows too.

During testing a 32-bit build issue was exposed in block/nfs.c and
the fix is included in this series.


Bin Meng (7):
  scripts/nsis.py: Drop the unnecessary path separator
  scripts/nsis.py: Fix destination directory name when invoked on
Windows
  scripts/nsis.py: Automatically package required DLLs of QEMU
executables
  .gitlab-ci.d/windows.yml: Drop the sed processing in the 64-bit build
  block/nfs: Fix 32-bit Windows build
  .gitlab-ci.d/windows.yml: Unify the prerequisite packages
  .gitlab-ci.d/windows.yml: Test 'make installer' in the CI

 meson.build  |  1 +
 block/nfs.c  |  8 ++
 .gitlab-ci.d/windows.yml | 40 ---
 scripts/nsis.py  | 60 +---
 4 files changed, 89 insertions(+), 20 deletions(-)

-- 
2.34.1




[PATCH 5/7] block/nfs: Fix 32-bit Windows build

2022-09-08 Thread Bin Meng
From: Bin Meng 

libnfs.h declares nfs_fstat() as the following for win32:

  int nfs_fstat(struct nfs_context *nfs, struct nfsfh *nfsfh,
struct __stat64 *st);

The 'st' parameter should be of type 'struct __stat64'. The
codes happen to build successfully for 64-bit Windows, but it
does not build for 32-bit Windows.

Fixes: 6542aa9c75bc ("block: add native support for NFS")
Fixes: 18a8056e0bc7 ("block/nfs: cache allocated filesize for read-only files")
Signed-off-by: Bin Meng 
---

 block/nfs.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index 444c40b458..d5d67937dd 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -418,7 +418,11 @@ static int64_t nfs_client_open(NFSClient *client, 
BlockdevOptionsNfs *opts,
int flags, int open_flags, Error **errp)
 {
 int64_t ret = -EINVAL;
+#ifdef _WIN32
+struct __stat64 st;
+#else
 struct stat st;
+#endif
 char *file = NULL, *strp = NULL;
 
 qemu_mutex_init(&client->mutex);
@@ -781,7 +785,11 @@ static int nfs_reopen_prepare(BDRVReopenState *state,
   BlockReopenQueue *queue, Error **errp)
 {
 NFSClient *client = state->bs->opaque;
+#ifdef _WIN32
+struct __stat64 st;
+#else
 struct stat st;
+#endif
 int ret = 0;
 
 if (state->flags & BDRV_O_RDWR && bdrv_is_read_only(state->bs)) {
-- 
2.34.1




Re: [PATCH] migrate block dirty bitmap: Fix the block dirty bitmap can't to migration_completion when pending size larger than threshold size : 1、dirty bitmap size big enough (such as 8MB) when bloc

2022-09-08 Thread Markus Armbruster
Your subject line is way too long.




Re: [PATCH v3] 9pfs: use GHashTable for fid table

2022-09-08 Thread Linus Heckemann
(sorry for the dup @Greg, forgot to reply-all)

Greg Kurz  writes:
>> > g_hash_table_steal_extended() [1] actually allows to do just that.
>> 
>> g_hash_table_steal_extended unfortunately isn't available since it was
>> introduced in glib 2.58 and we're maintaining compatibility to 2.56.
>> 
>
> Ha... this could be addressed through conditional compilation, e.g.:

It still won't compile, because we set GLIB_VERSION_MAX_ALLOWED in
glib-compat.h and it would require a compat wrapper as described
there. I think that's a bit much for this far more marginal performance
change. I'm happy to resubmit with the TODO comment though if you like?



Re: [PATCH v3] 9pfs: use GHashTable for fid table

2022-09-08 Thread Greg Kurz
On Thu, 08 Sep 2022 18:10:28 +0200
Linus Heckemann  wrote:

> (sorry for the dup @Greg, forgot to reply-all)
> 
> Greg Kurz  writes:
> >> > g_hash_table_steal_extended() [1] actually allows to do just that.
> >> 
> >> g_hash_table_steal_extended unfortunately isn't available since it was
> >> introduced in glib 2.58 and we're maintaining compatibility to 2.56.
> >> 
> >
> > Ha... this could be addressed through conditional compilation, e.g.:
> 
> It still won't compile, because we set GLIB_VERSION_MAX_ALLOWED in
> glib-compat.h and it would require a compat wrapper as described

ah drat, you're right !

> there. I think that's a bit much for this far more marginal performance
> change. I'm happy to resubmit with the TODO comment though if you like?

Either that or Christian may add it when merging.

Cheers,

--
Greg



Re: [PATCH 6/6] parallels: Image repairing in parallels_open()

2022-09-08 Thread Denis V. Lunev

On 9/2/22 10:53, Alexander Ivanov wrote:

Repair an image at opening if the image is unclean or
out-of-image corruption was detected.

Signed-off-by: Alexander Ivanov 
---
  block/parallels.c | 95 ---
  1 file changed, 65 insertions(+), 30 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 08526196da..a7c3af4ef2 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -735,6 +735,18 @@ static int coroutine_fn 
parallels_co_check(BlockDriverState *bs,
  return ret;
  }
  
+typedef struct ParallelsOpenCheckCo {

+BlockDriverState *bs;
+BdrvCheckResult *res;
+BdrvCheckMode fix;
+int ret;
+} ParallelsOpenCheckCo;
+
+static void coroutine_fn parallels_co_open_check(void *opaque)
+{
+ParallelsOpenCheckCo *poc = opaque;
+poc->ret = parallels_co_check(poc->bs, poc->res, poc->fix);
+}
  
  static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,

  Error **errp)
@@ -947,8 +959,8 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
  {
  BDRVParallelsState *s = bs->opaque;
  ParallelsHeader ph;
-int ret, size, i;
-int64_t file_size;
+int ret, size;
+int64_t file_size, high_off;
  QemuOpts *opts = NULL;
  Error *local_err = NULL;
  char *buf;
@@ -1027,34 +1039,6 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
  }
  s->bat_bitmap = (uint32_t *)(s->header + 1);
  
-for (i = 0; i < s->bat_size; i++) {

-int64_t off = bat2sect(s, i);
-if (off >= file_size) {
-if (flags & BDRV_O_CHECK) {
-continue;
-}
-error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry "
-   "is larger than file size (%" PRIi64 ")",
-   off, i, file_size);
-ret = -EINVAL;
-goto fail;
-}
-if (off >= s->data_end) {
-s->data_end = off + s->tracks;
-}
-}
-
-if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
-/* Image was not closed correctly. The check is mandatory */
-s->header_unclean = true;
-if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
-error_setg(errp, "parallels: Image was not closed correctly; "
-   "cannot be opened read/write");
-ret = -EACCES;
-goto fail;
-}
-}
-
  opts = qemu_opts_create(¶llels_runtime_opts, NULL, 0, errp);
  if (!opts) {
  goto fail_options;
@@ -1116,7 +1100,58 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
  error_free(s->migration_blocker);
  goto fail;
  }
+
  qemu_co_mutex_init(&s->lock);
+
+if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
+s->header_unclean = true;
+}
+
+high_off = highest_offset(s) >> BDRV_SECTOR_BITS;
+if (high_off >= s->data_end) {
+s->data_end = high_off + s->tracks;
+}
+
+/*
+ * We don't repair the image here if it is opened for checks.
+ * Also let to work with images in RO mode.
+ */
+if ((flags & BDRV_O_CHECK) || !(flags & BDRV_O_RDWR)) {
+return 0;
+}
+
+/*
+ * Repair the image if it's dirty or
+ * out-of-image corruption was detected.
+ */
+if (s->data_end > file_size ||
+le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
+BdrvCheckResult res = {0};
+Coroutine *co;
+ParallelsOpenCheckCo poc = {
+.bs = bs,
+.res = &res,
+.fix = BDRV_FIX_ERRORS | BDRV_FIX_LEAKS,
+.ret = -EINPROGRESS
+};
+
+if (qemu_in_coroutine()) {
+/* From bdrv_co_create.  */
+parallels_co_open_check(&poc);
+} else {
+assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+co = qemu_coroutine_create(parallels_co_open_check, &poc);
+qemu_coroutine_enter(co);
+BDRV_POLL_WHILE(bs, poc.ret == -EINPROGRESS);
+}
+
+if (poc.ret < 0) {
+error_setg_errno(errp, -poc.ret,
+ "Could not repair corrupted image");
+goto fail;
+}
+}
+

bdrv_check() is your friend. No need to duplicate the code



  return 0;
  
  fail_format:





[PATCH] migrate block dirty bitmap: Fix the block dirty bitmap can't to migration_completion when pending size larger than threshold size

2022-09-08 Thread liuhaiwei
From: liuhaiwei 

1、dirty bitmap size big enough (such as 8MB) when block size 1T ;
2、we set the migrate speed or the bandwith is small enough(such as 4MB/s)
so we set the fake pending size when pending size > threshold size

Signed-off-by: liuhaiwei 
Signed-off-by: liuhaiwei 
---
 migration/block-dirty-bitmap.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 9aba7d9c22..40f5a1aaf9 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -782,7 +782,10 @@ static void dirty_bitmap_save_pending(QEMUFile *f, void 
*opaque,
 }
 
 qemu_mutex_unlock_iothread();
-
+/*we set the fake pending size  when the dirty bitmap size more than 
max_size */
+if(pending > max_size && max_size != 0){
+pending = max_size - 1;
+}
 trace_dirty_bitmap_save_pending(pending, max_size);
 
 *res_postcopy_only += pending;
-- 
2.27.0




[PATCH] block: use the request length for iov alignment

2022-09-08 Thread Keith Busch
From: Keith Busch 

An iov length needs to be aligned to the logical block size, which may
be larger than the memory alignment.

Signed-off-by: Keith Busch 
---
 block/io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/io.c b/block/io.c
index 0a8cbefe86..296d4b49a7 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3243,13 +3243,14 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov)
 {
 int i;
 size_t alignment = bdrv_min_mem_align(bs);
+size_t len = bs->bl.request_alignment;
 IO_CODE();
 
 for (i = 0; i < qiov->niov; i++) {
 if ((uintptr_t) qiov->iov[i].iov_base % alignment) {
 return false;
 }
-if (qiov->iov[i].iov_len % alignment) {
+if (qiov->iov[i].iov_len % len) {
 return false;
 }
 }
-- 
2.30.2




[PATCH] migrate block dirty bitmap: Fix the block dirty bitmap can't to migration_completion when pending size larger than threshold size : 1、dirty bitmap size big enough (such as 8MB) when block si

2022-09-08 Thread liuhaiwei
From: liuhaiwei 

so we set the fake pending size when pending size > threshold size

Signed-off-by: liuhaiwei 
Signed-off-by: liuhaiwei 
---
 migration/block-dirty-bitmap.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 9aba7d9c22..6086d8d1c3 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -782,7 +782,10 @@ static void dirty_bitmap_save_pending(QEMUFile *f, void 
*opaque,
 }
 
 qemu_mutex_unlock_iothread();
-
+/*we set the fake pending size  when the dirty bitmap size more than 
max_size(bandwith of speed) */
+if(pending > max_size && max_size == 0){
+pending = max_size - 1;
+}
 trace_dirty_bitmap_save_pending(pending, max_size);
 
 *res_postcopy_only += pending;
-- 
2.27.0




Re: [PATCH 3/6] parallels: Add checking and repairing duplicate offsets in BAT

2022-09-08 Thread Denis V. Lunev

On 9/2/22 10:52, Alexander Ivanov wrote:

Cluster offsets must be unique among all BAT entries.
Find duplicate offsets in the BAT.

If a duplicated offset is found fix it by copying the content
of the relevant cluster to a new allocated cluster and
set the new cluster offset to the duplicated entry.

Add host_cluster_index() helper to deduplicate the code.
Add highest_offset() helper. It will be used for code deduplication
in the next patch.

Signed-off-by: Alexander Ivanov 
---
  block/parallels.c | 136 ++
  1 file changed, 136 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index dbcaf5d310..339ce45634 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -136,6 +136,26 @@ static int cluster_remainder(BDRVParallelsState *s, 
int64_t sector_num,
  return MIN(nb_sectors, ret);
  }
  
+static uint32_t host_cluster_index(BDRVParallelsState *s, int64_t off)

+{
+off -= s->header->data_off << BDRV_SECTOR_BITS;
+return off / s->cluster_size;
+}
+
+static int64_t highest_offset(BDRVParallelsState *s)
+{
+int64_t off, high_off = 0;
+int i;
+
+for (i = 0; i < s->bat_size; i++) {
+off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+if (off > high_off) {
+high_off = off;
+}
+}
+return high_off;
+}
+
  static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
  int nb_sectors, int *pnum)
  {
@@ -547,6 +567,114 @@ static int parallels_check_leak(BlockDriverState *bs,
  return 0;
  }
  
+static int parallels_check_duplicate(BlockDriverState *bs,

+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+BDRVParallelsState *s = bs->opaque;
+QEMUIOVector qiov;
+int64_t off, high_off, sector;
+unsigned long *bitmap;
+uint32_t i, bitmap_size, cluster_index;
+int n, ret = 0;
+uint64_t *buf = NULL;
+bool new_allocations = false;
+
+high_off = highest_offset(s);
+if (high_off == 0) {
+return 0;
+}
+
+/*
+ * Create a bitmap of used clusters.
+ * If a bit is set, there is a BAT entry pointing to this cluster.
+ * Loop through the BAT entrues, check bits relevant to an entry offset.
+ * If bit is set, this entry is duplicated. Otherwise set the bit.
+ */
+bitmap_size = host_cluster_index(s, high_off) + 1;
+bitmap = bitmap_new(bitmap_size);
+
+buf = g_malloc(s->cluster_size);
+qemu_iovec_init(&qiov, 0);
+qemu_iovec_add(&qiov, buf, s->cluster_size);
+
+for (i = 0; i < s->bat_size; i++) {
+off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+if (off == 0) {
+continue;
+}
+
+cluster_index = host_cluster_index(s, off);
+if (test_bit(cluster_index, bitmap)) {
+/* this cluster duplicates another one */
+fprintf(stderr,
+"%s duplicate offset in BAT entry %u\n",
+fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
+
+res->corruptions++;
+
+if (fix & BDRV_FIX_ERRORS) {
+/*
+ * Reset the entry and allocate a new cluster
+ * for the relevant guest offset. In this way we let
+ * the lower layer to place the new cluster properly.
+ * Copy the original cluster to the allocated one.
+ */
+parallels_set_bat_entry(s, i, 0);
+
+ret = bdrv_pread(bs->file, off, s->cluster_size, buf, 0);
+if (ret < 0) {
+res->check_errors++;
+goto out;
+}
+
+sector = (i * s->cluster_size) >> BDRV_SECTOR_BITS;
+off = allocate_clusters(bs, sector, s->tracks, &n);
+if (off < 0) {
+res->check_errors++;
+ret = off;
+goto out;
+}
+off <<= BDRV_SECTOR_BITS;
+if (off > high_off) {
+high_off = off;
+}
+
+ret = bdrv_co_pwritev(bs->file, off, s->cluster_size, &qiov, 
0);
+if (ret < 0) {
+res->check_errors++;
+goto out;
+}
+
+new_allocations = true;
+res->corruptions_fixed++;
+}
+
+} else {
+bitmap_set(bitmap, cluster_index, 1);
+}
+}
+
+if (new_allocations) {
+/*
+ * When new clusters are allocated, file size increases
+ * by 128 Mb blocks. We need to truncate the file to the
+ * right size.
+ */
+ret = parallels_handle_leak(bs, res, high_off, true);
+if (ret < 0) {
+res->check_errors++;
+goto out;
+}
+}

OK. I have re-read the code with test case handy and now
understand the situatio

Re: [PATCH 3/6] parallels: Add checking and repairing duplicate offsets in BAT

2022-09-08 Thread Denis V. Lunev

On 9/8/22 19:15, Denis V. Lunev wrote:

On 9/2/22 10:52, Alexander Ivanov wrote:

Cluster offsets must be unique among all BAT entries.
Find duplicate offsets in the BAT.

If a duplicated offset is found fix it by copying the content
of the relevant cluster to a new allocated cluster and
set the new cluster offset to the duplicated entry.

Add host_cluster_index() helper to deduplicate the code.
Add highest_offset() helper. It will be used for code deduplication
in the next patch.

Signed-off-by: Alexander Ivanov 
---
  block/parallels.c | 136 ++
  1 file changed, 136 insertions(+)

diff --git a/block/parallels.c b/block/parallels.c
index dbcaf5d310..339ce45634 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -136,6 +136,26 @@ static int cluster_remainder(BDRVParallelsState 
*s, int64_t sector_num,

  return MIN(nb_sectors, ret);
  }
  +static uint32_t host_cluster_index(BDRVParallelsState *s, int64_t 
off)

+{
+    off -= s->header->data_off << BDRV_SECTOR_BITS;
+    return off / s->cluster_size;
+}
+
+static int64_t highest_offset(BDRVParallelsState *s)
+{
+    int64_t off, high_off = 0;
+    int i;
+
+    for (i = 0; i < s->bat_size; i++) {
+    off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+    if (off > high_off) {
+    high_off = off;
+    }
+    }
+    return high_off;
+}
+
  static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
  int nb_sectors, int *pnum)
  {
@@ -547,6 +567,114 @@ static int 
parallels_check_leak(BlockDriverState *bs,

  return 0;
  }
  +static int parallels_check_duplicate(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+    BDRVParallelsState *s = bs->opaque;
+    QEMUIOVector qiov;
+    int64_t off, high_off, sector;
+    unsigned long *bitmap;
+    uint32_t i, bitmap_size, cluster_index;
+    int n, ret = 0;
+    uint64_t *buf = NULL;
+    bool new_allocations = false;
+
+    high_off = highest_offset(s);
+    if (high_off == 0) {
+    return 0;
+    }
+
+    /*
+ * Create a bitmap of used clusters.
+ * If a bit is set, there is a BAT entry pointing to this cluster.
+ * Loop through the BAT entrues, check bits relevant to an entry 
offset.

+ * If bit is set, this entry is duplicated. Otherwise set the bit.
+ */
+    bitmap_size = host_cluster_index(s, high_off) + 1;
+    bitmap = bitmap_new(bitmap_size);
+
+    buf = g_malloc(s->cluster_size);
+    qemu_iovec_init(&qiov, 0);
+    qemu_iovec_add(&qiov, buf, s->cluster_size);
+
+    for (i = 0; i < s->bat_size; i++) {
+    off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+    if (off == 0) {
+    continue;
+    }
+
+    cluster_index = host_cluster_index(s, off);
+    if (test_bit(cluster_index, bitmap)) {
+    /* this cluster duplicates another one */
+    fprintf(stderr,
+    "%s duplicate offset in BAT entry %u\n",
+    fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
+
+    res->corruptions++;
+
+    if (fix & BDRV_FIX_ERRORS) {
+    /*
+ * Reset the entry and allocate a new cluster
+ * for the relevant guest offset. In this way we let
+ * the lower layer to place the new cluster properly.
+ * Copy the original cluster to the allocated one.
+ */
+    parallels_set_bat_entry(s, i, 0);
+
+    ret = bdrv_pread(bs->file, off, s->cluster_size, 
buf, 0);

+    if (ret < 0) {
+    res->check_errors++;
+    goto out;
+    }
+
+    sector = (i * s->cluster_size) >> BDRV_SECTOR_BITS;
+    off = allocate_clusters(bs, sector, s->tracks, &n);
+    if (off < 0) {
+    res->check_errors++;
+    ret = off;
+    goto out;
+    }
+    off <<= BDRV_SECTOR_BITS;
+    if (off > high_off) {
+    high_off = off;
+    }
+
+    ret = bdrv_co_pwritev(bs->file, off, 
s->cluster_size, &qiov, 0);

+    if (ret < 0) {
+    res->check_errors++;
+    goto out;
+    }
+
+    new_allocations = true;
+    res->corruptions_fixed++;
+    }
+
+    } else {
+    bitmap_set(bitmap, cluster_index, 1);
+    }
+    }
+
+    if (new_allocations) {
+    /*
+ * When new clusters are allocated, file size increases
+ * by 128 Mb blocks. We need to truncate the file to the
+ * right size.
+ */
+    ret = parallels_handle_leak(bs, res, high_off, true);
+    if (ret < 0) {
+    res->check_errors++;
+    goto out;
+    }
+    }

OK. I have re-read the code with t