Re: [PATCH v2 4/4] vga/cirrus: deprecate, don't build by default

2024-05-30 Thread Gerd Hoffmann
  Hi,

> > > static const TypeInfo cirrus_vga_info = {
> > > diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
> > > index 84be51670ed8..3abbf490 100644
> > > --- a/hw/display/cirrus_vga_isa.c
> > > +++ b/hw/display/cirrus_vga_isa.c
> > > @@ -85,6 +85,7 @@ static void isa_cirrus_vga_class_init(ObjectClass
> > > *klass, void *data)
> > >     dc->realize = isa_cirrus_vga_realizefn;
> > >     device_class_set_props(dc, isa_cirrus_vga_properties);
> > >     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
> > > +    klass->deprecation_note = "use stdvga instead";
> > 
> > Excepr some old OSes work better with this than stdvga so could this be
> > left and not removed? Does it cause a lot of work to keep this device? I
> > thought it's stable already and were not many changes for it lately. If
> > something works why drop it?
> 
> Seconded: whilst stdvga is preferred, there are a lot of older OSs that work
> well in QEMU using the Cirrus emulation. I appreciate that the code could do
> with a bit of work, but is there a more specific reason that it should be
> deprecated?

Well, the cirrus has a 2d blitter implementation which I'd rate
problematic from both security and correctness point of view.

Also any guest new enough to still receive security updates can surely
use stdvga instead.  The "operating system museum" is IMHO pretty much
the only use case where it possibly might make sense to continue using
cirrus.

Having sayed that maybe the boolean classification -- be deprecated
or not -- is too simple.  The number of devices we can actually
deprecate and remove is probably relatively small.  But there also is
a large number of old-ish devices which only make sense in very few use
cases.  When running an old OS.  Or when emulating an old board.  Which
also tend to be not maintained very well because there are not many
users.  Maybe we need an "unsupported" state for them, with some
infrastructure like an easy way to compile qemu without unsupported
devices and an option to get warnings from qemu in case an unsupported
device is used.

take care,
  Gerd




[PATCH v2 4/4] vga/cirrus: deprecate, don't build by default

2024-05-30 Thread Gerd Hoffmann
stdvga is the much better option.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/cirrus_vga.c | 1 +
 hw/display/cirrus_vga_isa.c | 1 +
 hw/display/Kconfig  | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 150883a97166..81421be1f89d 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -3007,6 +3007,7 @@ static void cirrus_vga_class_init(ObjectClass *klass, 
void *data)
 dc->vmsd = _pci_cirrus_vga;
 device_class_set_props(dc, pci_vga_cirrus_properties);
 dc->hotpluggable = false;
+klass->deprecation_note = "use stdvga instead";
 }
 
 static const TypeInfo cirrus_vga_info = {
diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
index 84be51670ed8..3abbf490 100644
--- a/hw/display/cirrus_vga_isa.c
+++ b/hw/display/cirrus_vga_isa.c
@@ -85,6 +85,7 @@ static void isa_cirrus_vga_class_init(ObjectClass *klass, 
void *data)
 dc->realize = isa_cirrus_vga_realizefn;
 device_class_set_props(dc, isa_cirrus_vga_properties);
 set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+klass->deprecation_note = "use stdvga instead";
 }
 
 static const TypeInfo isa_cirrus_vga_info = {
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index a4552c8ed78d..cd0779396890 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -11,7 +11,6 @@ config FW_CFG_DMA
 
 config VGA_CIRRUS
 bool
-default y if PCI_DEVICES
 depends on PCI
 select VGA
 
-- 
2.45.1




[PATCH v2 1/4] qom: allow to mark objects (including devices) as deprecated.

2024-05-30 Thread Gerd Hoffmann
Add deprecation_note field (string) to ObjectClass.
Add deprecated bool to ObjectTypeInfo, report in 'qom-list-types'.
Print the note when listing devices via '-device help'.

Signed-off-by: Gerd Hoffmann 
---
 include/qom/object.h  | 1 +
 qom/qom-qmp-cmds.c| 4 
 system/qdev-monitor.c | 5 +
 qapi/qom.json | 4 +++-
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 13d3a655ddf9..6c682aa0135f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -136,6 +136,7 @@ struct ObjectClass
 ObjectUnparent *unparent;
 
 GHashTable *properties;
+const char *deprecation_note;
 };
 
 /**
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index e91a2353472a..43de9c9ae141 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -101,6 +101,10 @@ static void qom_list_types_tramp(ObjectClass *klass, void 
*data)
 if (parent) {
 info->parent = g_strdup(object_class_get_name(parent));
 }
+if (klass->deprecation_note) {
+info->has_deprecated = true;
+info->deprecated = true;
+}
 
 QAPI_LIST_PREPEND(*pret, info);
 }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d667f..704be312e1a7 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -144,6 +144,8 @@ static bool qdev_class_has_alias(DeviceClass *dc)
 
 static void qdev_print_devinfo(DeviceClass *dc)
 {
+ObjectClass *klass = OBJECT_CLASS(dc);
+
 qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
 if (dc->bus_type) {
 qemu_printf(", bus %s", dc->bus_type);
@@ -157,6 +159,9 @@ static void qdev_print_devinfo(DeviceClass *dc)
 if (!dc->user_creatable) {
 qemu_printf(", no-user");
 }
+if (klass->deprecation_note) {
+qemu_printf(", deprecated \"%s\"", klass->deprecation_note);
+}
 qemu_printf("\n");
 }
 
diff --git a/qapi/qom.json b/qapi/qom.json
index 38dde6d785ac..bd062feabaf7 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -163,10 +163,12 @@
 #
 # @parent: Name of parent type, if any (since 2.10)
 #
+# @deprecated: the type is deprecated (since 9.1)
+#
 # Since: 1.1
 ##
 { 'struct': 'ObjectTypeInfo',
-  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
+  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str', 
'*deprecated': 'bool' } }
 
 ##
 # @qom-list-types:
-- 
2.45.1




[PATCH v2 3/4] usb/hub: deprecate, don't build by default

2024-05-30 Thread Gerd Hoffmann
The hub supports only USB 1.1.  When running out of usb ports it is in
almost all cases the much better choice to add another usb host adapter
(or increase the number of root ports when using xhci) instead of using
the usb hub.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/dev-hub.c | 1 +
 hw/usb/Kconfig   | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 06e9537d0356..68444d39534f 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -686,6 +686,7 @@ static void usb_hub_class_initfn(ObjectClass *klass, void 
*data)
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 dc->fw_name = "hub";
 dc->vmsd = _usb_hub;
+klass->deprecation_note = "use more root ports or additional hostadapters 
instead";
 device_class_set_props(dc, usb_hub_properties);
 }
 
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 84bc7fbe36cd..b583f5c25d05 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -67,7 +67,6 @@ config TUSB6010
 
 config USB_HUB
 bool
-default y
 depends on USB
 
 config USB_HID
-- 
2.45.1




[PATCH v2 0/4] allow to deprecate objects and devices

2024-05-30 Thread Gerd Hoffmann
Put some infrastructure in place to allow tagging objects (including
devices) as deprected.  Use it to mark the ohci pci host adapter and
the usb hub as deprecated.

v2:
 - pick up reviews.
 - drop ohci patch.
 - add cirrus vga patch.

Gerd Hoffmann (4):
  qom: allow to mark objects (including devices) as deprecated.
  usb: add config options for the hub and hid devices
  usb/hub: deprecate, don't build by default
  vga/cirrus: deprecate, don't build by default

 include/qom/object.h| 1 +
 hw/display/cirrus_vga.c | 1 +
 hw/display/cirrus_vga_isa.c | 1 +
 hw/usb/dev-hub.c| 1 +
 qom/qom-qmp-cmds.c  | 4 
 system/qdev-monitor.c   | 5 +
 hw/display/Kconfig  | 1 -
 hw/usb/Kconfig  | 9 +
 hw/usb/meson.build  | 4 ++--
 qapi/qom.json   | 4 +++-
 10 files changed, 27 insertions(+), 4 deletions(-)

-- 
2.45.1




[PATCH v2 2/4] usb: add config options for the hub and hid devices

2024-05-30 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Thomas Huth 
---
 hw/usb/Kconfig | 10 ++
 hw/usb/meson.build |  4 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index f569ed7eeaa1..84bc7fbe36cd 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -65,6 +65,16 @@ config TUSB6010
 bool
 select USB_MUSB
 
+config USB_HUB
+bool
+default y
+depends on USB
+
+config USB_HID
+bool
+default y
+depends on USB
+
 config USB_TABLET_WACOM
 bool
 default y
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 23f7f7acb50b..d7de1003e3db 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -35,8 +35,8 @@ system_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: 
files('xlnx-versal-usb2-ctrl-
 system_ss.add(when: 'CONFIG_XLNX_USB_SUBSYS', if_true: 
files('xlnx-usb-subsystem.c'))
 
 # emulated usb devices
-system_ss.add(when: 'CONFIG_USB', if_true: files('dev-hub.c'))
-system_ss.add(when: 'CONFIG_USB', if_true: files('dev-hid.c'))
+system_ss.add(when: 'CONFIG_USB_HUB', if_true: files('dev-hub.c'))
+system_ss.add(when: 'CONFIG_USB_HID', if_true: files('dev-hid.c'))
 system_ss.add(when: 'CONFIG_USB_TABLET_WACOM', if_true: files('dev-wacom.c'))
 system_ss.add(when: 'CONFIG_USB_STORAGE_CORE', if_true: files('dev-storage.c'))
 system_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: 
files('dev-storage-bot.c'))
-- 
2.45.1




[PATCH v2] vnc: increase max display size

2024-05-30 Thread Gerd Hoffmann
It's 2024.  4k display resolutions are a thing these days.
Raise width and height limits of the qemu vnc server.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1596
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.h b/ui/vnc.h
index 4521dc88f792..e5fa2efa3e5d 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -81,8 +81,8 @@ typedef void VncSendHextileTile(VncState *vs,
 
 /* VNC_MAX_WIDTH must be a multiple of VNC_DIRTY_PIXELS_PER_BIT. */
 
-#define VNC_MAX_WIDTH ROUND_UP(2560, VNC_DIRTY_PIXELS_PER_BIT)
-#define VNC_MAX_HEIGHT 2048
+#define VNC_MAX_WIDTH ROUND_UP(5120, VNC_DIRTY_PIXELS_PER_BIT)
+#define VNC_MAX_HEIGHT 2160
 
 /* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
 #define VNC_DIRTY_BITS (VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT)
-- 
2.45.1




[PATCH] stdvga: fix screen blanking

2024-05-30 Thread Gerd Hoffmann
In case the display surface uses a shared buffer (i.e. uses vga vram
directly instead of a shadow) go unshare the buffer before clearing it.

This avoids vga memory corruption, which in turn fixes unblanking not
working properly with X11.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2067
Signed-off-by: Gerd Hoffmann 
---
 hw/display/vga.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 30facc6c8e33..34ab8eb9b745 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1762,6 +1762,12 @@ static void vga_draw_blank(VGACommonState *s, int 
full_update)
 if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
 return;
 
+if (is_buffer_shared(surface)) {
+/* unshare buffer, otherwise the blanking corrupts vga vram */
+qemu_console_resize(s->con, s->last_scr_width, s->last_scr_height);
+surface = qemu_console_surface(s->con);
+}
+
 w = s->last_scr_width * surface_bytes_per_pixel(surface);
 d = surface_data(surface);
 for(i = 0; i < s->last_scr_height; i++) {
-- 
2.45.1




Re: [PATCH v2 1/4] MAINTAINERS: drop audio maintainership

2024-05-28 Thread Gerd Hoffmann
  Hi,

> >  virtio-snd
> > -M: Gerd Hoffmann 
> > -R: Manos Pitsidianakis 
> > +M: Manos Pitsidianakis 
> > +R: Matias Ezequiel Vara Larsen 
> >  S: Supported
> >  F: hw/audio/virtio-snd.c
> >  F: hw/audio/virtio-snd-pci.c
> 
> While extra reviewers are always helpful, someone like Volker would
> make sense, not someone without any contributions:

Matias volunteered to help (via reply to v1 of the series), and for
'reviewer' role I don't see a reason to be strict.  'Maintainer' would
be a different story of course.

If Volker wants step up (I see you cc'ed him already) I happily add
him too.

take care,
  Gerd




[PATCH 4/4] usb/hub: deprecate, don't build by default

2024-05-28 Thread Gerd Hoffmann
The hub supports only USB 1.1.  When running out of usb ports it is in
almost all cases the much better choice to add another usb host adapter
(or increase the number of root ports when using xhci) instead of using
the usb hub.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/dev-hub.c | 1 +
 hw/usb/Kconfig   | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 06e9537d0356..68444d39534f 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -686,6 +686,7 @@ static void usb_hub_class_initfn(ObjectClass *klass, void 
*data)
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 dc->fw_name = "hub";
 dc->vmsd = _usb_hub;
+klass->deprecation_note = "use more root ports or additional hostadapters 
instead";
 device_class_set_props(dc, usb_hub_properties);
 }
 
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index c4a6ea5a687f..a8644c43296b 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -66,7 +66,6 @@ config TUSB6010
 
 config USB_HUB
 bool
-default y
 depends on USB
 
 config USB_HID
-- 
2.45.1




[PATCH 0/4] allow to deprecate objects and devices

2024-05-28 Thread Gerd Hoffmann
Put some infrastructure in place to allow tagging objects (including
devices) as deprected.  Use it to mark the ohci pci host adapter and
the usb hub as deprecated.

Gerd Hoffmann (4):
  qom: allow to mark objects (including devices) as deprecated.
  usb: add config options for the hub and hid devices
  usb/ohci-pci: deprecate, don't build by default
  usb/hub: deprecate, don't build by default

 include/qom/object.h  |  1 +
 hw/usb/dev-hub.c  |  1 +
 hw/usb/hcd-ohci-pci.c |  1 +
 qom/qom-qmp-cmds.c|  4 
 system/qdev-monitor.c |  5 +
 hw/usb/Kconfig| 10 +-
 hw/usb/meson.build|  4 ++--
 qapi/qom.json |  4 +++-
 8 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.45.1




[PATCH 1/4] qom: allow to mark objects (including devices) as deprecated.

2024-05-28 Thread Gerd Hoffmann
Add deprecation_note field (string) to ObjectClass.
Add deprecated bool to ObjectTypeInfo, report in 'qom-list-types'.
Print the note when listing devices via '-device help'.

Signed-off-by: Gerd Hoffmann 
---
 include/qom/object.h  | 1 +
 qom/qom-qmp-cmds.c| 4 
 system/qdev-monitor.c | 5 +
 qapi/qom.json | 4 +++-
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 13d3a655ddf9..6c682aa0135f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -136,6 +136,7 @@ struct ObjectClass
 ObjectUnparent *unparent;
 
 GHashTable *properties;
+const char *deprecation_note;
 };
 
 /**
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index e91a2353472a..43de9c9ae141 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -101,6 +101,10 @@ static void qom_list_types_tramp(ObjectClass *klass, void 
*data)
 if (parent) {
 info->parent = g_strdup(object_class_get_name(parent));
 }
+if (klass->deprecation_note) {
+info->has_deprecated = true;
+info->deprecated = true;
+}
 
 QAPI_LIST_PREPEND(*pret, info);
 }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d667f..704be312e1a7 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -144,6 +144,8 @@ static bool qdev_class_has_alias(DeviceClass *dc)
 
 static void qdev_print_devinfo(DeviceClass *dc)
 {
+ObjectClass *klass = OBJECT_CLASS(dc);
+
 qemu_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
 if (dc->bus_type) {
 qemu_printf(", bus %s", dc->bus_type);
@@ -157,6 +159,9 @@ static void qdev_print_devinfo(DeviceClass *dc)
 if (!dc->user_creatable) {
 qemu_printf(", no-user");
 }
+if (klass->deprecation_note) {
+qemu_printf(", deprecated \"%s\"", klass->deprecation_note);
+}
 qemu_printf("\n");
 }
 
diff --git a/qapi/qom.json b/qapi/qom.json
index 38dde6d785ac..bd062feabaf7 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -163,10 +163,12 @@
 #
 # @parent: Name of parent type, if any (since 2.10)
 #
+# @deprecated: the type is deprecated (since 9.1)
+#
 # Since: 1.1
 ##
 { 'struct': 'ObjectTypeInfo',
-  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
+  'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str', 
'*deprecated': 'bool' } }
 
 ##
 # @qom-list-types:
-- 
2.45.1




[PATCH 3/4] usb/ohci-pci: deprecate, don't build by default

2024-05-28 Thread Gerd Hoffmann
The xhci host adapter is the much better choice.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-ohci-pci.c | 1 +
 hw/usb/Kconfig| 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ohci-pci.c b/hw/usb/hcd-ohci-pci.c
index 33ed9b6f5a52..88de657def71 100644
--- a/hw/usb/hcd-ohci-pci.c
+++ b/hw/usb/hcd-ohci-pci.c
@@ -143,6 +143,7 @@ static void ohci_pci_class_init(ObjectClass *klass, void 
*data)
 dc->hotpluggable = false;
 dc->vmsd = _ohci;
 dc->reset = usb_ohci_reset_pci;
+klass->deprecation_note = "use qemu-xhci instead";
 }
 
 static const TypeInfo ohci_pci_info = {
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 84bc7fbe36cd..c4a6ea5a687f 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -17,7 +17,6 @@ config USB_OHCI_SYSBUS
 
 config USB_OHCI_PCI
 bool
-default y if PCI_DEVICES
 depends on PCI
 select USB_OHCI
 
-- 
2.45.1




[PATCH 2/4] usb: add config options for the hub and hid devices

2024-05-28 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/Kconfig | 10 ++
 hw/usb/meson.build |  4 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index f569ed7eeaa1..84bc7fbe36cd 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -65,6 +65,16 @@ config TUSB6010
 bool
 select USB_MUSB
 
+config USB_HUB
+bool
+default y
+depends on USB
+
+config USB_HID
+bool
+default y
+depends on USB
+
 config USB_TABLET_WACOM
 bool
 default y
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 23f7f7acb50b..d7de1003e3db 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -35,8 +35,8 @@ system_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: 
files('xlnx-versal-usb2-ctrl-
 system_ss.add(when: 'CONFIG_XLNX_USB_SUBSYS', if_true: 
files('xlnx-usb-subsystem.c'))
 
 # emulated usb devices
-system_ss.add(when: 'CONFIG_USB', if_true: files('dev-hub.c'))
-system_ss.add(when: 'CONFIG_USB', if_true: files('dev-hid.c'))
+system_ss.add(when: 'CONFIG_USB_HUB', if_true: files('dev-hub.c'))
+system_ss.add(when: 'CONFIG_USB_HID', if_true: files('dev-hid.c'))
 system_ss.add(when: 'CONFIG_USB_TABLET_WACOM', if_true: files('dev-wacom.c'))
 system_ss.add(when: 'CONFIG_USB_STORAGE_CORE', if_true: files('dev-storage.c'))
 system_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: 
files('dev-storage-bot.c'))
-- 
2.45.1




[PATCH v2 1/4] MAINTAINERS: drop audio maintainership

2024-05-28 Thread Gerd Hoffmann
Remove myself from audio (both devices and backend) entries.
Flip status to "Orphan" for entries which have nobody else listed.

Cc: Manos Pitsidianakis 
Cc: Matias Ezequiel Vara Larsen 
Cc: Thomas Huth 
Signed-off-by: Gerd Hoffmann 
---
 MAINTAINERS | 30 ++
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 448dc951c509..58e44885ce94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1913,8 +1913,7 @@ F: include/hw/xtensa/mx_pic.h
 Devices
 ---
 Overall Audio frontends
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: hw/audio/
 F: include/hw/audio/
 F: tests/qtest/ac97-test.c
@@ -2389,8 +2388,8 @@ F: hw/virtio/virtio-mem-pci.c
 F: include/hw/virtio/virtio-mem.h
 
 virtio-snd
-M: Gerd Hoffmann 
-R: Manos Pitsidianakis 
+M: Manos Pitsidianakis 
+R: Matias Ezequiel Vara Larsen 
 S: Supported
 F: hw/audio/virtio-snd.c
 F: hw/audio/virtio-snd-pci.c
@@ -2768,7 +2767,6 @@ F: include/hw/hyperv/hv-balloon.h
 Subsystems
 --
 Overall Audio backends
-M: Gerd Hoffmann 
 M: Marc-André Lureau 
 S: Odd Fixes
 F: audio/
@@ -2784,13 +2782,11 @@ X: audio/spiceaudio.c
 F: qapi/audio.json
 
 ALSA Audio backend
-M: Gerd Hoffmann 
 R: Christian Schoenebeck 
-S: Odd Fixes
+S: Orphan
 F: audio/alsaaudio.c
 
 Core Audio framework backend
-M: Gerd Hoffmann 
 M: Philippe Mathieu-Daudé 
 R: Christian Schoenebeck 
 R: Akihiko Odaki 
@@ -2798,36 +2794,30 @@ S: Odd Fixes
 F: audio/coreaudio.c
 
 DSound Audio backend
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: audio/dsound*
 
 JACK Audio Connection Kit backend
-M: Gerd Hoffmann 
 R: Christian Schoenebeck 
-S: Odd Fixes
+S: Orphan
 F: audio/jackaudio.c
 
 Open Sound System (OSS) Audio backend
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: audio/ossaudio.c
 
 PulseAudio backend
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: audio/paaudio.c
 
 SDL Audio backend
-M: Gerd Hoffmann 
-R: Thomas Huth 
+M: Thomas Huth 
 S: Odd Fixes
 F: audio/sdlaudio.c
 
 Sndio Audio backend
-M: Gerd Hoffmann 
 R: Alexandre Ratchov 
-S: Odd Fixes
+S: Orphan
 F: audio/sndioaudio.c
 
 Block layer core
-- 
2.45.1




[PATCH v2 2/4] MAINTAINERS: drop usb maintainership

2024-05-28 Thread Gerd Hoffmann
Remove myself from usb entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Manos Pitsidianakis 
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 58e44885ce94..a03756274017 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2140,8 +2140,7 @@ F: tests/qtest/fuzz-sdcard-test.c
 F: tests/qtest/sdhci-test.c
 
 USB
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: hw/usb/*
 F: stubs/usb-dev-stub.c
 F: tests/qtest/usb-*-test.c
@@ -2150,7 +2149,6 @@ F: include/hw/usb.h
 F: include/hw/usb/
 
 USB (serial adapter)
-R: Gerd Hoffmann 
 M: Samuel Thibault 
 S: Maintained
 F: hw/usb/dev-serial.c
-- 
2.45.1




[PATCH v2 4/4] MAINTAINERS: drop spice+ui maintainership

2024-05-28 Thread Gerd Hoffmann
Remove myself from spice and ui entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Manos Pitsidianakis 
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 37f2be570cc7..8479e8146470 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3044,8 +3044,7 @@ F: stubs/memory_device.c
 F: docs/nvdimm.txt
 
 SPICE
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: include/ui/qemu-spice.h
 F: include/ui/spice-display.h
 F: ui/spice-*.c
@@ -3055,7 +3054,6 @@ F: qapi/ui.json
 F: docs/spice-port-fqdn.txt
 
 Graphics
-M: Gerd Hoffmann 
 M: Marc-André Lureau 
 S: Odd Fixes
 F: ui/
-- 
2.45.1




[PATCH v2 3/4] MAINTAINERS: drop virtio-gpu maintainership

2024-05-28 Thread Gerd Hoffmann
Remove myself from virtio-gpu entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Manos Pitsidianakis 
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a03756274017..37f2be570cc7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2574,8 +2574,7 @@ F: hw/display/ramfb*.c
 F: include/hw/display/ramfb.h
 
 virtio-gpu
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: hw/display/virtio-gpu*
 F: hw/display/virtio-vga.*
 F: include/hw/virtio/virtio-gpu.h
@@ -2597,7 +2596,6 @@ F: include/hw/virtio/virtio-blk-common.h
 
 vhost-user-gpu
 M: Marc-André Lureau 
-R: Gerd Hoffmann 
 S: Maintained
 F: docs/interop/vhost-user-gpu.rst
 F: contrib/vhost-user-gpu
-- 
2.45.1




[PATCH v2 0/4] MAINTAINERS: update kraxel's entries.

2024-05-28 Thread Gerd Hoffmann
I have not found much time to work on qemu due to being busy with
firmware (edk2 for the most part).  Time to update the MAINTAINERS
file entries to match reality.

I drop spice, ui, audio and usb due to lack of time.

I drop virtio-gpu, I don't follow recent development (venus etc.)
close enough to be able to actually review the changes.  Looking at
qemu-devel traffic for virtio-gpu it seems to be in good hands with
multiple people working on it, even though this is not reflected in
the MAINTAINERS file.

I keep the firmware bits (edk2, fw_cfg).

I also keep some other pieces which don't see much development
activity such as stdvga and cirrus for now.  I might revisit
this later.

v2 changes:
 - flip entries without maintainer to orphan even if there is
   a reviewer left.
 - add/upgrade volunteers from replies to audio sections.

take care,
  Gerd

Gerd Hoffmann (4):
  MAINTAINERS: drop audio maintainership
  MAINTAINERS: drop usb maintainership
  MAINTAINERS: drop virtio-gpu maintainership
  MAINTAINERS: drop spice+ui maintainership

 MAINTAINERS | 42 +-
 1 file changed, 13 insertions(+), 29 deletions(-)

-- 
2.45.1




[PATCH v5] hw/pflash: fix block write start

2024-05-16 Thread Gerd Hoffmann
Move the pflash_blk_write_start() call.  We need the offset of the
first data write, not the offset for the setup (number-of-bytes)
write.  Without this fix u-boot can do block writes to the first
flash block only.

While being at it drop a leftover FIXME.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2343
Fixes: 284a7ee2e290 ("hw/pflash: implement update buffer for block writes")
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi01.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 1bda8424b907..c8f1cf5a8722 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -518,10 +518,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 break;
 case 0xe8: /* Write to buffer */
 trace_pflash_write(pfl->name, "write to buffer");
-/* FIXME should save @offset, @width for case 1+ */
-qemu_log_mask(LOG_UNIMP,
-  "%s: Write to buffer emulation is flawed\n",
-  __func__);
 pfl->status |= 0x80; /* Ready! */
 break;
 case 0xf0: /* Probe for AMD flash */
@@ -574,7 +570,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 }
 pfl->counter = value;
 pfl->wcycle++;
-pflash_blk_write_start(pfl, offset);
 break;
 case 0x60:
 if (cmd == 0xd0) {
@@ -605,6 +600,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 switch (pfl->cmd) {
 case 0xe8: /* Block write */
 /* FIXME check @offset, @width */
+if (pfl->blk_offset == -1 && pfl->counter) {
+pflash_blk_write_start(pfl, offset);
+}
 if (!pfl->ro && (pfl->blk_offset != -1)) {
 pflash_data_write(pfl, offset, value, width, be);
 } else {
-- 
2.45.0




[PATCH 4/4] MAINTAINERS: drop spice+ui maintainership

2024-05-16 Thread Gerd Hoffmann
Remove myself from spice and ui entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4d9f4fd09823..d5b6a1c76abb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3042,8 +3042,7 @@ F: stubs/memory_device.c
 F: docs/nvdimm.txt
 
 SPICE
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: include/ui/qemu-spice.h
 F: include/ui/spice-display.h
 F: ui/spice-*.c
@@ -3053,7 +3052,6 @@ F: qapi/ui.json
 F: docs/spice-port-fqdn.txt
 
 Graphics
-M: Gerd Hoffmann 
 M: Marc-André Lureau 
 S: Odd Fixes
 F: ui/
-- 
2.45.0




[PATCH 3/4] MAINTAINERS: drop virtio-gpu maintainership

2024-05-16 Thread Gerd Hoffmann
Remove myself from virtio-gpu entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d81376f84746..4d9f4fd09823 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2572,8 +2572,7 @@ F: hw/display/ramfb*.c
 F: include/hw/display/ramfb.h
 
 virtio-gpu
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: hw/display/virtio-gpu*
 F: hw/display/virtio-vga.*
 F: include/hw/virtio/virtio-gpu.h
@@ -2595,7 +2594,6 @@ F: include/hw/virtio/virtio-blk-common.h
 
 vhost-user-gpu
 M: Marc-André Lureau 
-R: Gerd Hoffmann 
 S: Maintained
 F: docs/interop/vhost-user-gpu.rst
 F: contrib/vhost-user-gpu
-- 
2.45.0




[PATCH 2/4] MAINTAINERS: drop usb maintainership

2024-05-16 Thread Gerd Hoffmann
Remove myself from usb entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
---
 MAINTAINERS | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f52e2912fc3..d81376f84746 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2140,8 +2140,7 @@ F: tests/qtest/fuzz-sdcard-test.c
 F: tests/qtest/sdhci-test.c
 
 USB
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: hw/usb/*
 F: stubs/usb-dev-stub.c
 F: tests/qtest/usb-*-test.c
@@ -2150,7 +2149,6 @@ F: include/hw/usb.h
 F: include/hw/usb/
 
 USB (serial adapter)
-R: Gerd Hoffmann 
 M: Samuel Thibault 
 S: Maintained
 F: hw/usb/dev-serial.c
-- 
2.45.0




[PATCH 1/4] MAINTAINERS: drop audio maintainership

2024-05-16 Thread Gerd Hoffmann
Remove myself from audio (both devices and backend) entries.
Flip status to "Orphan" for entries which have nobody else listed.

Signed-off-by: Gerd Hoffmann 
---
 MAINTAINERS | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1b79767d6196..7f52e2912fc3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1913,8 +1913,7 @@ F: include/hw/xtensa/mx_pic.h
 Devices
 ---
 Overall Audio frontends
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: hw/audio/
 F: include/hw/audio/
 F: tests/qtest/ac97-test.c
@@ -2388,7 +2387,6 @@ F: hw/virtio/virtio-mem-pci.c
 F: include/hw/virtio/virtio-mem.h
 
 virtio-snd
-M: Gerd Hoffmann 
 R: Manos Pitsidianakis 
 S: Supported
 F: hw/audio/virtio-snd.c
@@ -2767,7 +2765,6 @@ F: include/hw/hyperv/hv-balloon.h
 Subsystems
 --
 Overall Audio backends
-M: Gerd Hoffmann 
 M: Marc-André Lureau 
 S: Odd Fixes
 F: audio/
@@ -2783,13 +2780,11 @@ X: audio/spiceaudio.c
 F: qapi/audio.json
 
 ALSA Audio backend
-M: Gerd Hoffmann 
 R: Christian Schoenebeck 
 S: Odd Fixes
 F: audio/alsaaudio.c
 
 Core Audio framework backend
-M: Gerd Hoffmann 
 M: Philippe Mathieu-Daudé 
 R: Christian Schoenebeck 
 R: Akihiko Odaki 
@@ -2797,34 +2792,28 @@ S: Odd Fixes
 F: audio/coreaudio.c
 
 DSound Audio backend
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: audio/dsound*
 
 JACK Audio Connection Kit backend
-M: Gerd Hoffmann 
 R: Christian Schoenebeck 
 S: Odd Fixes
 F: audio/jackaudio.c
 
 Open Sound System (OSS) Audio backend
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: audio/ossaudio.c
 
 PulseAudio backend
-M: Gerd Hoffmann 
-S: Odd Fixes
+S: Orphan
 F: audio/paaudio.c
 
 SDL Audio backend
-M: Gerd Hoffmann 
 R: Thomas Huth 
 S: Odd Fixes
 F: audio/sdlaudio.c
 
 Sndio Audio backend
-M: Gerd Hoffmann 
 R: Alexandre Ratchov 
 S: Odd Fixes
 F: audio/sndioaudio.c
-- 
2.45.0




[PATCH 0/4] MAINTAINERS: update kraxel's entries.

2024-05-16 Thread Gerd Hoffmann
I have not found much time to work on qemu due to being busy with
firmware (edk2 for the most part).  Time to update the MAINTAINERS
file entries to match reality.

I drop spice, ui, audio and usb due to lack of time.

I drop virtio-gpu, I don't follow recent development (venus etc.)
close enough to be able to actually review the changes.  Looking at
qemu-devel traffic for virtio-gpu it seems to be in good hands with
multiple people working on it, even though this is not reflected in
the MAINTAINERS file.

I keep the firmware bits (edk2, fw_cfg).

I also keep some other pieces which don't see much development
activity such as stdvga and cirrus for now.  I might revisit
this later.

take care,
  Gerd

Gerd Hoffmann (4):
  MAINTAINERS: drop audio maintainership
  MAINTAINERS: drop usb maintainership
  MAINTAINERS: drop virtio-gpu maintainership
  MAINTAINERS: drop spice+ui maintainership

 MAINTAINERS | 31 +++
 1 file changed, 7 insertions(+), 24 deletions(-)

-- 
2.45.0




[PATCH v4] hw/pflash: fix block write start

2024-05-16 Thread Gerd Hoffmann
Move the pflash_blk_write_start() call.  We need the offset of the
first data write, not the offset for the setup (number-of-bytes)
write.  Without this fix u-boot can do block writes to the first
flash block only.

While being at it drop a leftover FIXME.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2343
Fixes: fcc79f2e0955 ("hw/pflash: implement update buffer for block writes")
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi01.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 1bda8424b907..c8f1cf5a8722 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -518,10 +518,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 break;
 case 0xe8: /* Write to buffer */
 trace_pflash_write(pfl->name, "write to buffer");
-/* FIXME should save @offset, @width for case 1+ */
-qemu_log_mask(LOG_UNIMP,
-  "%s: Write to buffer emulation is flawed\n",
-  __func__);
 pfl->status |= 0x80; /* Ready! */
 break;
 case 0xf0: /* Probe for AMD flash */
@@ -574,7 +570,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 }
 pfl->counter = value;
 pfl->wcycle++;
-pflash_blk_write_start(pfl, offset);
 break;
 case 0x60:
 if (cmd == 0xd0) {
@@ -605,6 +600,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 switch (pfl->cmd) {
 case 0xe8: /* Block write */
 /* FIXME check @offset, @width */
+if (pfl->blk_offset == -1 && pfl->counter) {
+pflash_blk_write_start(pfl, offset);
+}
 if (!pfl->ro && (pfl->blk_offset != -1)) {
 pflash_data_write(pfl, offset, value, width, be);
 } else {
-- 
2.45.0




[PATCH v3] hw/pflash: fix block write start

2024-05-15 Thread Gerd Hoffmann
Move the pflash_blk_write_start() call.  We need the offset of the
first data write, not the offset for the setup (number-of-bytes)
write.  Without this fix u-boot can do block writes to the first
flash block only.

While being at it drop a leftover FIXME.

Resolves: #2343
Fixes: fcc79f2e0955 ("hw/pflash: implement update buffer for block writes")
Signed-off-by: Gerd Hoffmann 
---
 hw/block/pflash_cfi01.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 1bda8424b907..c8f1cf5a8722 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -518,10 +518,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 break;
 case 0xe8: /* Write to buffer */
 trace_pflash_write(pfl->name, "write to buffer");
-/* FIXME should save @offset, @width for case 1+ */
-qemu_log_mask(LOG_UNIMP,
-  "%s: Write to buffer emulation is flawed\n",
-  __func__);
 pfl->status |= 0x80; /* Ready! */
 break;
 case 0xf0: /* Probe for AMD flash */
@@ -574,7 +570,6 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 }
 pfl->counter = value;
 pfl->wcycle++;
-pflash_blk_write_start(pfl, offset);
 break;
 case 0x60:
 if (cmd == 0xd0) {
@@ -605,6 +600,9 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 switch (pfl->cmd) {
 case 0xe8: /* Block write */
 /* FIXME check @offset, @width */
+if (pfl->blk_offset == -1 && pfl->counter) {
+pflash_blk_write_start(pfl, offset);
+}
 if (!pfl->ro && (pfl->blk_offset != -1)) {
 pflash_data_write(pfl, offset, value, width, be);
 } else {
-- 
2.45.0




Re: [RFC PATCH 0/1] pci: allocate a PCI ID for RISC-V IOMMU

2024-05-07 Thread Gerd Hoffmann
On Tue, May 07, 2024 at 11:37:05PM GMT, Frank Chang wrote:
> Hi Daniel,
> 
> Daniel Henrique Barboza  於 2024年5月3日 週五 下午8:43寫道:
> >
> > Hi,
> >
> > In this RFC I want to check with Gerd and others if it's ok to add a PCI
> > id for the RISC-V IOMMU device. It's currently under review in [1]. The
> 
> Is the link [1] missing?

Yes ;)

Also:  A bit more background on the iommu would be great, for example a
pointer to the specification.

take care,
  Gerd




Re: Problems (timeouts) when testing usb-ohci with qemu

2024-04-24 Thread Gerd Hoffmann
> qemu hack:
> 
>  hw/usb/hcd-ohci.c | 11 +++
>  hw/usb/hcd-ohci.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index fc8fc91a1d..99e52ad13a 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -267,6 +267,10 @@ static inline void ohci_intr_update(OHCIState *ohci)
>  (ohci->intr_status & ohci->intr))
>  level = 1;
>  
> +if (level && ohci->level)
> +qemu_set_irq(ohci->irq, 0);
> +
> +ohci->level = level;
>  qemu_set_irq(ohci->irq, level);
>  }
>  
> diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
> index e1827227ac..6f82e72bd9 100644
> --- a/hw/usb/hcd-ohci.h
> +++ b/hw/usb/hcd-ohci.h
> @@ -52,6 +52,7 @@ struct OHCIState {
>  uint32_t ctl, status;
>  uint32_t intr_status;
>  uint32_t intr;
> +int level;
>  
>  /* memory pointer partition */
>  uint32_t hcca;

Phew.  Disclaimer: Havn't looked at the ohci emulation code for years.

It should not be needed to store the interrupt level that way.  It is
possible to calculate what the interrupt level should be, based on the
interrupt status register and the interrupt mask register, and the code
above seems to do exactly that (the "ohci->intr_status & ohci->intr"
bit).

ohci_intr_update() must be called if one of these two registers changes,
i.e. if the guest changes the mask, if the guest acks an IRQ by clearing
an status bit, if the device raises an IRQ by setting an status bit.
Might be the ohci emulation has a bug here.

Another possible trouble spot is that the timing behavior is different
on virtual vs. physical hardware.  Specifically with the emulated
hardware some actions appear to complete instantly (when the vmexit to
handle the mmio register write returns it's finished already), which
will never complete that quickly on physical hardware.  So drivers can
have race conditions which only trigger on virtual hardware.  The error
pattern you are describing sounds like this could be the case here.

HTH & take care,
  Gerd




Re: [edk2-devel] [PATCH v3 5/6] target/arm: Do memory type alignment check when translation disabled

2024-04-19 Thread Gerd Hoffmann
  Hi,

> Gerd, any ideas?  Maybe I needs something subtly different in my
> edk2 build?  I've not looked at this bit of the qemu infrastructure
> before - is there a document on how that image is built?

There is roms/Makefile for that.

make -C roms help
make -C roms efi

So easiest would be to just update the edk2 submodule to what you
need, then rebuild.

The build is handled by the roms/edk2-build.py script,
with the build configuration being in roms/edk2-build.config.
That is usable outside the qemu source tree too, i.e. like this:

  python3 /path/to/qemu.git/roms/edk2-build.py \
--config /path/to/qemu.git/roms/edk2-build.config \
--core /path/to/edk2.git \
--match armvirt \
--silent --no-logs

That'll try to place the images build in "../pc-bios", so maybe better
work with a copy of the config file where you adjust this.

HTH,
  Gerd




Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

2024-04-15 Thread Gerd Hoffmann
  Hi,

> > Options I see:
> > 
> >   (a) Stop using direct kernel boot, let virt-install & other tools
> >   create vfat boot media with shim+kernel+initrd instead.
> > 
> >   (b) Enroll the distro signing keys in the efi variable store, so
> >   booting the kernel without shim.efi works.
> > 
> >   (c) Add support for loading shim to qemu (and ovmf), for example
> >   with a new '-shim' command line option which stores shim.efi
> >   in some new fw_cfg file.
> 
> The problem with this is that now virt-install  has to actually
> find the correct a shim.efi binary. It is already somewhat hard
> to find a suitable kerenl+initrd binary, and AFAIK, the places
> where we get these binaries don't have shim.efi alongside.
> 
> eg for RHEL/Fedora we grab kernel+initrd from the pxeboot dir:
> 
>   
> https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/images/pxeboot/

shim is 
https://fedora.mirrorservice.org/fedora/linux/development/rawhide/Everything/x86_64/os/EFI/BOOT/BOOTX64.EFI

> In various forums we have discussed adding the secureboot
> certs to the libosinfo database, so that we can have a
> customized EFI varstore with minimized certs, even for the
> ISO / HDD boot scenario.

Well.  It's not that easy unfortunately.  At least the "minimized certs"
part.  shim often is signed with the microsoft keys only, so you can't
drop that without rendering the install.iso unbootable.

Only adding the distro certs without removing the microsoft certs works
of course.

> If we do that, then (b) is trivial for direct kernel boot too.

Yep.

> (b) kills all birds with the same stone :-)

See above.  I'd love this being true but it is not.

> > (b) + (c) both require a fix for the patching issue.  The options
> > I see here are:
> > 
> >   (A) Move the patching from qemu to the linuxboot option rom.
> >   Strictly speaking it belongs there anyway.  It doesn't look
> >   that easy though, for qemu it is easier to gather all
> >   information needed ...
> > 
> >   (B) Provide both patched and unpatched setup header, so the
> >   guest can choose what it needs.
> > 
> >   (C) When implementing (c) above we can piggyback on the -shim
> >   switch and skip patching in case it is present.
> > 
> >   (D) Add a flag to skip the patching.
> > 
> > Comments?  Other/better ideas?
> 
> I guess (b) + (D) is probably my preference.

I prefer (B) over (D) because that doesn't require a new config option
(which probably needs support in libvirt and possibly higher up in the
management stack too ...).

Patch series implementing (B) and the -shim switch:
https://lore.kernel.org/qemu-devel/20240411094830.1337658-1-kra...@redhat.com/

Using -shim is optional, so it's up to virt-install whenever it wants go
for (b) or (c).

take care,
  Gerd




Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()

2024-04-11 Thread Gerd Hoffmann
On Thu, Apr 11, 2024 at 11:36:10AM +0200, Philippe Mathieu-Daudé wrote:
> On 11/4/24 09:47, Gerd Hoffmann wrote:
> >Hi,
> > 
> > >  Due to security concerns inherent in the design of sprintf(3),
> > >  it is highly recommended that you use snprintf(3) instead.
> > 
> > > -char response[40];
> > > +g_autofree char *response = NULL;
> > 
> > > -sprintf(response, "\033[%d;%dR",
> > > +response = g_strdup_printf("\033[%d;%dR",
> > 
> > Any specific reason why you don't go with the recommendation above?
> > 
> > While using g_strdup_printf() isn't wrong it allocates memory which
> > is not needed here because you can continue to use the stack buffer
> > this way:
> > 
> > snprintf(response, sizeof(response), ...);
> 
> I thought GLib/GString was recommended for formatting,

If you allocate the output buffer anyway (and there are patches in this
series where this is the case) it's clearly better to use
g_strdup_printf instead of malloc + snprintf.

In case a fixed-size buffer can be used I wouldn't switch to dynamic
allocation ...

take care,
  Gerd




[PATCH 5/5] x86/loader: add -shim option

2024-04-11 Thread Gerd Hoffmann
Add new -shim command line option, wire up for the x86 loader.
When specified load shim into the new "etc/boot/shim" fw_cfg file.

Needs OVMF changes too to be actually useful.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/boards.h |  1 +
 hw/core/machine.c   | 20 
 hw/i386/x86.c   | 16 
 system/vl.c |  9 +
 qemu-options.hx |  7 +++
 5 files changed, 53 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 8b8f6d5c00d3..37da417cb029 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -399,6 +399,7 @@ struct MachineState {
 BootConfiguration boot_config;
 char *kernel_filename;
 char *kernel_cmdline;
+char *shim_filename;
 char *initrd_filename;
 const char *cpu_type;
 AccelState *accelerator;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 37ede0e7d4fd..f27f6ae8e199 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -313,6 +313,21 @@ static void machine_set_kernel(Object *obj, const char 
*value, Error **errp)
 ms->kernel_filename = g_strdup(value);
 }
 
+static char *machine_get_shim(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return g_strdup(ms->shim_filename);
+}
+
+static void machine_set_shim(Object *obj, const char *value, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+g_free(ms->shim_filename);
+ms->shim_filename = g_strdup(value);
+}
+
 static char *machine_get_initrd(Object *obj, Error **errp)
 {
 MachineState *ms = MACHINE(obj);
@@ -988,6 +1003,11 @@ static void machine_class_init(ObjectClass *oc, void 
*data)
 object_class_property_set_description(oc, "kernel",
 "Linux kernel image file");
 
+object_class_property_add_str(oc, "shim",
+machine_get_shim, machine_set_shim);
+object_class_property_set_description(oc, "shim",
+"shim.efi file");
+
 object_class_property_add_str(oc, "initrd",
 machine_get_initrd, machine_set_initrd);
 object_class_property_set_description(oc, "initrd",
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6724e408e576..3e95f196fb40 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1128,6 +1128,22 @@ void x86_load_linux(X86MachineState *x86ms,
 /* kernel without setup header patches */
 fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
 
+if (machine->shim_filename) {
+GMappedFile *mapped_file;
+GError *gerr = NULL;
+
+mapped_file = g_mapped_file_new(machine->shim_filename, false, );
+if (!mapped_file) {
+fprintf(stderr, "qemu: error reading shim %s: %s\n",
+machine->shim_filename, gerr->message);
+exit(1);
+}
+
+fw_cfg_add_file(fw_cfg, "etc/boot/shim",
+g_mapped_file_get_contents(mapped_file),
+g_mapped_file_get_length(mapped_file));
+}
+
 if (sev_enabled()) {
 sev_add_kernel_loader_hashes(_load_ctx, _fatal);
 }
diff --git a/system/vl.c b/system/vl.c
index 0c6201c5bdc5..4df42ba8c7a6 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2407,6 +2407,7 @@ static void configure_accelerators(const char *progname)
 static void qemu_validate_options(const QDict *machine_opts)
 {
 const char *kernel_filename = qdict_get_try_str(machine_opts, "kernel");
+const char *shim_filename = qdict_get_try_str(machine_opts, "shim");
 const char *initrd_filename = qdict_get_try_str(machine_opts, "initrd");
 const char *kernel_cmdline = qdict_get_try_str(machine_opts, "append");
 
@@ -2416,6 +2417,11 @@ static void qemu_validate_options(const QDict 
*machine_opts)
 exit(1);
 }
 
+if (shim_filename != NULL) {
+error_report("-shim only allowed with -kernel option");
+exit(1);
+}
+
 if (initrd_filename != NULL) {
 error_report("-initrd only allowed with -kernel option");
 exit(1);
@@ -2908,6 +2914,9 @@ void qemu_init(int argc, char **argv)
 case QEMU_OPTION_kernel:
 qdict_put_str(machine_opts_dict, "kernel", optarg);
 break;
+case QEMU_OPTION_shim:
+qdict_put_str(machine_opts_dict, "shim", optarg);
+break;
 case QEMU_OPTION_initrd:
 qdict_put_str(machine_opts_dict, "initrd", optarg);
 break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 8ce85d45598d..b5151857afe5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4002,6 +4002,13 @@ SRST
 or in multiboot format.
 ERST
 
+DEF("shim", HAS_ARG, QEMU_OPTION_shim, \
+"-shim shim.efi use 'shim.efi' to boot the kernel\n", QEMU_ARCH_ALL)
+SRST
+``-shim shim.efi``
+Use 'shim.efi' to boot the kernel
+ERST
+
 DEF("append", HAS_ARG, QEMU_OPTION_append, \
 "-append cmdline use 'cmdline' as kernel command line\n", QEMU_ARCH_ALL)
 SRST
-- 
2.44.0




[PATCH 2/5] x86/loader: only patch linux kernels

2024-04-11 Thread Gerd Hoffmann
If the binary loaded via -kernel is *not* a linux kernel (in which
case protocol == 0), do not patch the linux kernel header fields.

It's (a) pointless and (b) might break binaries by random patching
and (c) changes the binary hash which in turn breaks secure boot
verification.

Background: OVMF happily loads and runs not only linux kernels but
any efi binary via direct kernel boot.

Note: Breaking the secure boot verification is a problem for linux
kernels too, but fixed that is left for another day ...

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ffbda48917fd..765899eebe43 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1108,7 +1108,7 @@ void x86_load_linux(X86MachineState *x86ms,
  * kernel on the other side of the fw_cfg interface matches the hash of the
  * file the user passed in.
  */
-if (!sev_enabled()) {
+if (!sev_enabled() && protocol > 0) {
 memcpy(setup, header, MIN(sizeof(header), setup_size));
 }
 
-- 
2.44.0




[PATCH 1/5] vl: fix qemu_validate_options() indention

2024-04-11 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 system/vl.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/system/vl.c b/system/vl.c
index c64422298245..0c6201c5bdc5 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2411,15 +2411,15 @@ static void qemu_validate_options(const QDict 
*machine_opts)
 const char *kernel_cmdline = qdict_get_try_str(machine_opts, "append");
 
 if (kernel_filename == NULL) {
- if (kernel_cmdline != NULL) {
-  error_report("-append only allowed with -kernel option");
-  exit(1);
-  }
+if (kernel_cmdline != NULL) {
+error_report("-append only allowed with -kernel option");
+exit(1);
+}
 
-  if (initrd_filename != NULL) {
-  error_report("-initrd only allowed with -kernel option");
-  exit(1);
-  }
+if (initrd_filename != NULL) {
+error_report("-initrd only allowed with -kernel option");
+exit(1);
+}
 }
 
 if (loadvm && incoming) {
-- 
2.44.0




[PATCH 4/5] x86/loader: expose unpatched kernel

2024-04-11 Thread Gerd Hoffmann
Add a new "etc/boot/kernel" fw_cfg file, containing the kernel without
the setup header patches.  Intended use is booting in UEFI with secure
boot enabled, where the setup header patching breaks secure boot
verification.

Needs OVMF changes too to be actually useful.

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/x86.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 6f75948b3021..6724e408e576 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1125,6 +1125,9 @@ void x86_load_linux(X86MachineState *x86ms,
 sev_load_ctx.setup_data = (char *)setup;
 sev_load_ctx.setup_size = setup_size;
 
+/* kernel without setup header patches */
+fw_cfg_add_file(fw_cfg, "etc/boot/kernel", kernel, kernel_size);
+
 if (sev_enabled()) {
 sev_add_kernel_loader_hashes(_load_ctx, _fatal);
 }
-- 
2.44.0




[PATCH 0/5] x86/loader: secure boot support for direct kernel load

2024-04-11 Thread Gerd Hoffmann
This series allows to boot linux kernels and other efi binaries via
direct kernel load with secure boot enabled.

The series adds two new fw_cfg files: 'etc/boot/kernel' contains the
kernel without modifications (no setup header patching), and
'etc/boot/shim' contains shim.

The path to the shim binary can be passed to qemu using the new '-shim'
command line switch.

This needs a companion patch series for tianocore which will put the new
fw_cfg files into use, a draft of that series can be found here:

https://github.com/kraxel/edk2/commits/devel/direct-secure-boot/

With everything in place it is possible to use direct kernel load with
secure boot enabled.

take care,
  Gerd

Gerd Hoffmann (5):
  vl: fix qemu_validate_options() indention
  x86/loader: only patch linux kernels
  x86/loader: read complete kernel
  x86/loader: expose unpatched kernel
  x86/loader: add -shim option

 include/hw/boards.h |  1 +
 hw/core/machine.c   | 20 
 hw/i386/x86.c   | 32 ++--
 system/vl.c | 25 +
 qemu-options.hx |  7 +++
 5 files changed, 71 insertions(+), 14 deletions(-)

-- 
2.44.0




[PATCH 3/5] x86/loader: read complete kernel

2024-04-11 Thread Gerd Hoffmann
Load the complete kernel (including setup) into memory.  Excluding the
setup is handled later when adding the FW_CFG_KERNEL_SIZE and
FW_CFG_KERNEL_DATA entries.

This is a preparation for the next patch which adds a new fw_cfg file
containing the complete, unpatched kernel.  No functional change.

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/x86.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 765899eebe43..6f75948b3021 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1058,7 +1058,6 @@ void x86_load_linux(X86MachineState *x86ms,
 fprintf(stderr, "qemu: invalid kernel header\n");
 exit(1);
 }
-kernel_size -= setup_size;
 
 setup  = g_malloc(setup_size);
 kernel = g_malloc(kernel_size);
@@ -1067,6 +1066,7 @@ void x86_load_linux(X86MachineState *x86ms,
 fprintf(stderr, "fread() failed\n");
 exit(1);
 }
+fseek(f, 0, SEEK_SET);
 if (fread(kernel, 1, kernel_size, f) != kernel_size) {
 fprintf(stderr, "fread() failed\n");
 exit(1);
@@ -1113,10 +1113,11 @@ void x86_load_linux(X86MachineState *x86ms,
 }
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
-fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, kernel, kernel_size);
-sev_load_ctx.kernel_data = (char *)kernel;
-sev_load_ctx.kernel_size = kernel_size;
+fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size - setup_size);
+fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
+ kernel + setup_size, kernel_size - setup_size);
+sev_load_ctx.kernel_data = (char *)kernel + setup_size;
+sev_load_ctx.kernel_size = kernel_size - setup_size;
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_ADDR, real_addr);
 fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, setup_size);
-- 
2.44.0




Re: [PATCH 01/12] ui/console-vc: Replace sprintf() by g_strdup_printf()

2024-04-11 Thread Gerd Hoffmann
  Hi,

> Due to security concerns inherent in the design of sprintf(3),
> it is highly recommended that you use snprintf(3) instead.

> -char response[40];
> +g_autofree char *response = NULL;

> -sprintf(response, "\033[%d;%dR",
> +response = g_strdup_printf("\033[%d;%dR",

Any specific reason why you don't go with the recommendation above?

While using g_strdup_printf() isn't wrong it allocates memory which
is not needed here because you can continue to use the stack buffer
this way:

snprintf(response, sizeof(response), ...);

take care,
  Gerd




Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

2024-04-10 Thread Gerd Hoffmann
> > > Options I see:
> > > 
> > >   (a) Stop using direct kernel boot, let virt-install & other tools
> > >   create vfat boot media with shim+kernel+initrd instead.
> > > 
> > >   (b) Enroll the distro signing keys in the efi variable store, so
> > >   booting the kernel without shim.efi works.
> > > 
> > >   (c) Add support for loading shim to qemu (and ovmf), for example
> > >   with a new '-shim' command line option which stores shim.efi
> > >   in some new fw_cfg file.
> > > 
> > > (b) + (c) both require a fix for the patching issue.  The options
> > > I see here are:
> > > 
> > >   (A) Move the patching from qemu to the linuxboot option rom.
> > >   Strictly speaking it belongs there anyway.  It doesn't look
> > >   that easy though, for qemu it is easier to gather all
> > >   information needed ...
> > > 
> > >   (B) Provide both patched and unpatched setup header, so the
> > >   guest can choose what it needs.
> > > 
> > >   (C) When implementing (c) above we can piggyback on the -shim
> > >   switch and skip patching in case it is present.
> > > 
> > >   (D) Add a flag to skip the patching.
> > > 
> > > Comments?  Other/better ideas?
> > > 
> > > take care,
> > >   Gerd
> > 
> > So if you didn't decide whether to do b or c then I guess D is
> > easiest and covers both cases?
> 
> Easiest if you look at qemu only.  Adding a new config option adds
> burdens elsewhere though.  Users and the management stack have to
> learn to use the new option.
> 
> Both (A) and (B) work automatically and can be combined with both (b)
> and (c).  (B) is probably much easier to implement, drawback is it
> requires an firmware update too.

Sneak preview for (c) + (B) is here:
  https://git.kraxel.org/cgit/qemu/log/?h=sirius/direct-secure-boot

(well, almost, instead of unpatched setup header it exposes an unpatched
kernel binary).

Currently looking at the ovmf side of things to make sure the idea
actually works before posting patches to the list.

take care,
  Gerd




Re: secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

2024-04-10 Thread Gerd Hoffmann
On Wed, Apr 10, 2024 at 07:10:22AM -0400, Michael S. Tsirkin wrote:
> On Wed, Apr 10, 2024 at 12:35:13PM +0200, Gerd Hoffmann wrote:
> > On Wed, Apr 10, 2024 at 03:26:29AM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> > > > If the binary loaded via -kernel is *not* a linux kernel (in which
> > > > case protocol == 0), do not patch the linux kernel header fields.
> > > > 
> > > > It's (a) pointless and (b) might break binaries by random patching
> > > > and (c) changes the binary hash which in turn breaks secure boot
> > > > verification.
> > > > 
> > > > Background: OVMF happily loads and runs not only linux kernels but
> > > > any efi binary via direct kernel boot.
> > > > 
> > > > Note: Breaking the secure boot verification is a problem for linux
> > > > kernels too, but fixed that is left for another day ...
> > > 
> > > Um we kind of care about Linux ;)
> > > 
> > > What's the plan?  I suspect we should just add a command line flag
> > > to skip patching? And once we do that, it seems safer to just
> > > always rely on the flag?
> > 
> > Well, there are more problems to solve here than just the patching.  So
> > lets have a look at the bigger picture before discussion the details ...
> > 
> > [ Cc'ing Daniel + Cole ]
> > 
> > Current state of affairs is that OVMF supports two ways to boot a linux
> > kernel:
> > 
> >  (1) Just load it as EFI binary and boot via linux kernel EFI stub,
> >  which is the modern way to load a linux kernel (which is why you
> >  can boot not only linux kernels but any efi binary).
> > 
> >  (2) Use the old EFI handover protocol.  Which is the RHEL-6 era way to
> >  boot a linux kernel on EFI.
> > 
> > For method (1) secure boot verification must pass.  For (2) not.  So if
> > you try to use direct kernel boot with secure boot enabled OVMF will
> > first try (1), which will fail, then go fallback to (2).
> > 
> > The reason for the failure is not only the patching, but also the fact
> > that the linux kernel is typically verified by shim.efi (and the distro
> > keys compiled into the binary) instead of the firmware.
> > 
> > Going though (2) is not ideal for multiple reasons, so we need some
> > strategy how we'll go handle direct kernel load with uefi and secure
> > boot in a way that (1) works.
> > 
> > Options I see:
> > 
> >   (a) Stop using direct kernel boot, let virt-install & other tools
> >   create vfat boot media with shim+kernel+initrd instead.
> > 
> >   (b) Enroll the distro signing keys in the efi variable store, so
> >   booting the kernel without shim.efi works.
> > 
> >   (c) Add support for loading shim to qemu (and ovmf), for example
> >   with a new '-shim' command line option which stores shim.efi
> >   in some new fw_cfg file.
> > 
> > (b) + (c) both require a fix for the patching issue.  The options
> > I see here are:
> > 
> >   (A) Move the patching from qemu to the linuxboot option rom.
> >   Strictly speaking it belongs there anyway.  It doesn't look
> >   that easy though, for qemu it is easier to gather all
> >   information needed ...
> > 
> >   (B) Provide both patched and unpatched setup header, so the
> >   guest can choose what it needs.
> > 
> >   (C) When implementing (c) above we can piggyback on the -shim
> >   switch and skip patching in case it is present.
> > 
> >   (D) Add a flag to skip the patching.
> > 
> > Comments?  Other/better ideas?
> > 
> > take care,
> >   Gerd
> 
> So if you didn't decide whether to do b or c then I guess D is
> easiest and covers both cases?

Easiest if you look at qemu only.  Adding a new config option adds
burdens elsewhere though.  Users and the management stack have to
learn to use the new option.

Both (A) and (B) work automatically and can be combined with both (b)
and (c).  (B) is probably much easier to implement, drawback is it
requires an firmware update too.

take care,
  Gerd




secure boot & direct kernel load (was: Re: [PATCH] x86/loader: only patch linux kernels)

2024-04-10 Thread Gerd Hoffmann
On Wed, Apr 10, 2024 at 03:26:29AM -0400, Michael S. Tsirkin wrote:
> On Wed, Apr 10, 2024 at 09:21:26AM +0200, Gerd Hoffmann wrote:
> > If the binary loaded via -kernel is *not* a linux kernel (in which
> > case protocol == 0), do not patch the linux kernel header fields.
> > 
> > It's (a) pointless and (b) might break binaries by random patching
> > and (c) changes the binary hash which in turn breaks secure boot
> > verification.
> > 
> > Background: OVMF happily loads and runs not only linux kernels but
> > any efi binary via direct kernel boot.
> > 
> > Note: Breaking the secure boot verification is a problem for linux
> > kernels too, but fixed that is left for another day ...
> 
> Um we kind of care about Linux ;)
> 
> What's the plan?  I suspect we should just add a command line flag
> to skip patching? And once we do that, it seems safer to just
> always rely on the flag?

Well, there are more problems to solve here than just the patching.  So
lets have a look at the bigger picture before discussion the details ...

[ Cc'ing Daniel + Cole ]

Current state of affairs is that OVMF supports two ways to boot a linux
kernel:

 (1) Just load it as EFI binary and boot via linux kernel EFI stub,
 which is the modern way to load a linux kernel (which is why you
 can boot not only linux kernels but any efi binary).

 (2) Use the old EFI handover protocol.  Which is the RHEL-6 era way to
 boot a linux kernel on EFI.

For method (1) secure boot verification must pass.  For (2) not.  So if
you try to use direct kernel boot with secure boot enabled OVMF will
first try (1), which will fail, then go fallback to (2).

The reason for the failure is not only the patching, but also the fact
that the linux kernel is typically verified by shim.efi (and the distro
keys compiled into the binary) instead of the firmware.

Going though (2) is not ideal for multiple reasons, so we need some
strategy how we'll go handle direct kernel load with uefi and secure
boot in a way that (1) works.

Options I see:

  (a) Stop using direct kernel boot, let virt-install & other tools
  create vfat boot media with shim+kernel+initrd instead.

  (b) Enroll the distro signing keys in the efi variable store, so
  booting the kernel without shim.efi works.

  (c) Add support for loading shim to qemu (and ovmf), for example
  with a new '-shim' command line option which stores shim.efi
  in some new fw_cfg file.

(b) + (c) both require a fix for the patching issue.  The options
I see here are:

  (A) Move the patching from qemu to the linuxboot option rom.
  Strictly speaking it belongs there anyway.  It doesn't look
  that easy though, for qemu it is easier to gather all
  information needed ...

  (B) Provide both patched and unpatched setup header, so the
  guest can choose what it needs.

  (C) When implementing (c) above we can piggyback on the -shim
  switch and skip patching in case it is present.

  (D) Add a flag to skip the patching.

Comments?  Other/better ideas?

take care,
  Gerd




[PATCH] x86/loader: only patch linux kernels

2024-04-10 Thread Gerd Hoffmann
If the binary loaded via -kernel is *not* a linux kernel (in which
case protocol == 0), do not patch the linux kernel header fields.

It's (a) pointless and (b) might break binaries by random patching
and (c) changes the binary hash which in turn breaks secure boot
verification.

Background: OVMF happily loads and runs not only linux kernels but
any efi binary via direct kernel boot.

Note: Breaking the secure boot verification is a problem for linux
kernels too, but fixed that is left for another day ...

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ffbda48917fd..765899eebe43 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1108,7 +1108,7 @@ void x86_load_linux(X86MachineState *x86ms,
  * kernel on the other side of the fw_cfg interface matches the hash of the
  * file the user passed in.
  */
-if (!sev_enabled()) {
+if (!sev_enabled() && protocol > 0) {
 memcpy(setup, header, MIN(sizeof(header), setup_size));
 }
 
-- 
2.44.0




Re: [PATCH] edk2: get version + date from git submodule

2024-04-09 Thread Gerd Hoffmann
On Tue, Apr 09, 2024 at 04:13:34PM +0100, Peter Maydell wrote:
> On Tue, 9 Apr 2024 at 15:19, Peter Maydell  wrote:
> >
> > On Tue, 9 Apr 2024 at 15:14, Gerd Hoffmann  wrote:
> > >
> > >   Hi,
> > >
> > > > > +   --version-override "$(EDK2_STABLE)-for-qemu" \
> > > > > +   --release-date "$(EDK2_DATE)" \
> > > >
> > > > Hi -- I've just noticed that we never made this change to
> > > > automate the date/version for EDK2 ROMs, but we also never
> > > > updated the version by hand. So at the moment we ship an
> > > > EDK2 blob that wrongly claims to be an older version.
> > > > See this bug report by a user:
> > > >
> > > > https://gitlab.com/qemu-project/qemu/-/issues/2233
> > > >
> > > > Is it possible to fix this for 9.0?
> > >
> > > I've posted v2 (series) a while back, no feedback so far.
> > >
> > > https://lore.kernel.org/qemu-devel/20240327102448.61877-1-kra...@redhat.com/
> > >
> > > If there are no objections I can do a PR for these three patches plus an
> > > edk2 binary rebuild (which shouldn't change anything but the version
> > > string).
> >
> > I guess that's safe enough, though the very-conservative
> > choice would be to take just the EDK2 rebuild for 9.0.
> 
> Would you be able to get a pullreq in for this before rc3?
> (I can delay rc3 by a day or so if necessary; I'd rather
> not have to do an rc4 if we can avoid it...)

https://lore.kernel.org/qemu-devel/20240409162942.454419-1-kra...@redhat.com/T/

take care,
  Gerd




[PULL 1/4] edk2: get version + date from git submodule

2024-04-09 Thread Gerd Hoffmann
Turned out hard-coding version and date in the Makefile wasn't a bright
idea.  Updating it on edk2 updates is easily forgotten.  Fetch the info
from git instead.  Store in edk2-version, so this can be committed to
the repo and is present in tarballs too.

Reviewed-by: Peter Maydell 
Signed-off-by: Gerd Hoffmann 
Message-ID: <20240327102448.61877-2-kra...@redhat.com>
---
 roms/Makefile | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index edc234a0e886..783a5cab4f4c 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -52,6 +52,8 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
 #
 EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
 
+-include edk2-version
+
 default help:
@echo "nothing is build by default"
@echo "available build targets:"
@@ -147,10 +149,19 @@ skiboot:
$(MAKE) -C skiboot CROSS=$(powerpc64_cross_prefix)
cp skiboot/skiboot.lid ../pc-bios/skiboot.lid
 
-efi:
+edk2-version: edk2
+   if test -e edk2/.git; then \
+   echo "EDK2_STABLE = $$(cd edk2; git describe --tags --match 
'edk2-stable*')" > $@; \
+   echo "EDK2_DATE = $$(cd edk2; git log -1 --pretty='format:%cd' 
--date='format:%m/%d/%Y')" >> $@; \
+   else \
+   touch $@; \
+   fi
+
+efi: edk2-version
$(PYTHON) edk2-build.py --config edk2-build.config \
-   --version-override "edk2-stable202302-for-qemu" \
-   --release-date "03/01/2023"
+   --version-override "$(EDK2_STABLE)-for-qemu" \
+   --release-date "$(EDK2_DATE)" \
+   --silent --no-logs
rm -f ../pc-bios/edk2-*.fd.bz2
bzip2 --verbose ../pc-bios/edk2-*.fd
 
-- 
2.44.0




[PULL 0/4] Edk2 20240409 patches

2024-04-09 Thread Gerd Hoffmann
The following changes since commit e5c6528dce86d7a9ada7ecf02fcb7b8560955131:

  Update version for v9.0.0-rc2 release (2024-04-02 20:59:43 +0100)

are available in the Git repository at:

  https://gitlab.com/kraxel/qemu.git tags/edk2-20240409-pull-request

for you to fetch changes up to e3404e01c7f74efdc3440ddfd339d67bf7a8410e:

  edk2: rebuild binaries with correct version information (2024-04-09 18:21:23 
+0200)


edk2: fix version information, rebuild binaries.



Gerd Hoffmann (4):
  edk2: get version + date from git submodule
  edk2: commit version info
  edk2/seabios: use common extra version
  edk2: rebuild binaries with correct version information

 pc-bios/edk2-aarch64-code.fd.bz2   | Bin 1589310 -> 1588976 bytes
 pc-bios/edk2-arm-code.fd.bz2   | Bin 1571693 -> 1571639 bytes
 pc-bios/edk2-i386-code.fd.bz2  | Bin 1775832 -> 1775230 bytes
 pc-bios/edk2-i386-secure-code.fd.bz2   | Bin 1876986 -> 1877268 bytes
 pc-bios/edk2-riscv-code.fd.bz2 | Bin 1289160 -> 1289337 bytes
 pc-bios/edk2-x86_64-code.fd.bz2| Bin 1892372 -> 1892766 bytes
 pc-bios/edk2-x86_64-microvm.fd.bz2 | Bin 1785258 -> 1785290 bytes
 pc-bios/edk2-x86_64-secure-code.fd.bz2 | Bin 2214892 -> 1969096 bytes
 roms/Makefile  |  25 ++---
 roms/edk2-version  |   2 ++
 10 files changed, 20 insertions(+), 7 deletions(-)
 create mode 100644 roms/edk2-version

-- 
2.44.0




[PULL 2/4] edk2: commit version info

2024-04-09 Thread Gerd Hoffmann
Reviewed-by: Peter Maydell 
Signed-off-by: Gerd Hoffmann 
Message-ID: <20240327102448.61877-3-kra...@redhat.com>
---
 roms/edk2-version | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 roms/edk2-version

diff --git a/roms/edk2-version b/roms/edk2-version
new file mode 100644
index ..1594ed8c4de9
--- /dev/null
+++ b/roms/edk2-version
@@ -0,0 +1,2 @@
+EDK2_STABLE = edk2-stable202402
+EDK2_DATE = 02/14/2024
-- 
2.44.0




[PULL 3/4] edk2/seabios: use common extra version

2024-04-09 Thread Gerd Hoffmann
Bring a bit more consistency into the naming.

Reviewed-by: Peter Maydell 
Signed-off-by: Gerd Hoffmann 
Message-ID: <20240327102448.61877-4-kra...@redhat.com>
---
 roms/Makefile | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 783a5cab4f4c..dfed2b216a1e 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -41,8 +41,8 @@ x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
 riscv32_cross_prefix := $(call find-cross-prefix,riscv32)
 riscv64_cross_prefix := $(call find-cross-prefix,riscv64)
 
-# tag our seabios builds
-SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
+# tag our firmware builds
+FIRMWARE_EXTRAVERSION = -prebuilt.qemu.org
 
 #
 # EfiRom utility is shipped with edk2 / tianocore, in BaseTools/
@@ -93,12 +93,12 @@ build-seabios-config-%: config.%
mkdir -p seabios/builds/$*
cp $< seabios/builds/$*/.config
$(MAKE) -C seabios \
-   EXTRAVERSION=$(SEABIOS_EXTRAVERSION) \
+   EXTRAVERSION=$(FIRMWARE_EXTRAVERSION) \
CROSS_PREFIX=$(x86_64_cross_prefix) \
KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \
OUT=$(CURDIR)/seabios/builds/$*/ oldnoconfig
$(MAKE) -C seabios \
-   EXTRAVERSION=$(SEABIOS_EXTRAVERSION) \
+   EXTRAVERSION=$(FIRMWARE_EXTRAVERSION) \
CROSS_PREFIX=$(x86_64_cross_prefix) \
KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \
OUT=$(CURDIR)/seabios/builds/$*/ all
@@ -159,7 +159,7 @@ edk2-version: edk2
 
 efi: edk2-version
$(PYTHON) edk2-build.py --config edk2-build.config \
-   --version-override "$(EDK2_STABLE)-for-qemu" \
+   --version-override "$(EDK2_STABLE)$(FIRMWARE_EXTRAVERSION)" \
--release-date "$(EDK2_DATE)" \
--silent --no-logs
rm -f ../pc-bios/edk2-*.fd.bz2
-- 
2.44.0




Re: [PATCH] edk2: get version + date from git submodule

2024-04-09 Thread Gerd Hoffmann
  Hi,

> > +   --version-override "$(EDK2_STABLE)-for-qemu" \
> > +   --release-date "$(EDK2_DATE)" \
> 
> Hi -- I've just noticed that we never made this change to
> automate the date/version for EDK2 ROMs, but we also never
> updated the version by hand. So at the moment we ship an
> EDK2 blob that wrongly claims to be an older version.
> See this bug report by a user:
> 
> https://gitlab.com/qemu-project/qemu/-/issues/2233
> 
> Is it possible to fix this for 9.0?

I've posted v2 (series) a while back, no feedback so far.

https://lore.kernel.org/qemu-devel/20240327102448.61877-1-kra...@redhat.com/

If there are no objections I can do a PR for these three patches plus an
edk2 binary rebuild (which shouldn't change anything but the version
string).

take care,
  Gerd




Re: [PATCH-for-9.0 0/4] hw/virtio: Protect from more DMA re-entrancy bugs

2024-04-05 Thread Gerd Hoffmann
On Thu, Apr 04, 2024 at 09:13:35PM +0200, Philippe Mathieu-Daudé wrote:
> Gerd suggested to use the transport guard to protect the
> device from DMA re-entrancy abuses.

Thanks for turning that idea into a proper patch series.

Series:
Reviewed-by: Gerd Hoffmann 

take care,
  Gerd




Re: [PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-04-02 Thread Gerd Hoffmann
On Fri, Mar 29, 2024 at 10:19:09AM +, Bernhard Beschow wrote:
> 
> In theory you could pass `-M acpi=off` to not instantiate the PIIX4
> ACPI function, essentially turning the Frankenstein-PIIX4 SB into a
> PIIX3. However, this also removes SMI registers used by SeaBIOS to
> handle SMM setup which may create unwanted side effects.

SeaBIOS will simply not use SMM in that case.

> On a real PIIX3, these registers are located in the ISA function. I
> wonder if it made sense to implement that for greater compatibility.
> 
> What do you think? Gerd, what do you think w.r.t. SeaBIOS?

Well, SMM support isn't that important I think.  It was introduced to
make switches to 32-bit mode more robust.  Entering SMM mode stores the
*complete* x86 processor state (which is impossible to do in other
ways), so with SMM it's possible to switch back into whatever state the
processor has been before entering 32-bit mode.

Some storage drivers (virtio, ahci) switch into 32-bit mode so they can
reach the mmio registers they need.  Some storage drivers (ide) don't,
in that case SMM doesn't change anything.

I'm not aware of any problems actually fixed by adding SMM support in
seabios.  I suspect the guest OS-es new enough to support ahci or virtio
are also new enough to not not call BIOS interfaces from non-standard
processor modes.

take care,
  Gerd




[PATCH v2 2/3] edk2: commit version info

2024-03-27 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 roms/edk2-version | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 roms/edk2-version

diff --git a/roms/edk2-version b/roms/edk2-version
new file mode 100644
index ..1594ed8c4de9
--- /dev/null
+++ b/roms/edk2-version
@@ -0,0 +1,2 @@
+EDK2_STABLE = edk2-stable202402
+EDK2_DATE = 02/14/2024
-- 
2.44.0




[PATCH v2 3/3] edk2/seabios: use common extra version

2024-03-27 Thread Gerd Hoffmann
Bring a bit more consistency into the naming.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 783a5cab4f4c..dfed2b216a1e 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -41,8 +41,8 @@ x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
 riscv32_cross_prefix := $(call find-cross-prefix,riscv32)
 riscv64_cross_prefix := $(call find-cross-prefix,riscv64)
 
-# tag our seabios builds
-SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
+# tag our firmware builds
+FIRMWARE_EXTRAVERSION = -prebuilt.qemu.org
 
 #
 # EfiRom utility is shipped with edk2 / tianocore, in BaseTools/
@@ -93,12 +93,12 @@ build-seabios-config-%: config.%
mkdir -p seabios/builds/$*
cp $< seabios/builds/$*/.config
$(MAKE) -C seabios \
-   EXTRAVERSION=$(SEABIOS_EXTRAVERSION) \
+   EXTRAVERSION=$(FIRMWARE_EXTRAVERSION) \
CROSS_PREFIX=$(x86_64_cross_prefix) \
KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \
OUT=$(CURDIR)/seabios/builds/$*/ oldnoconfig
$(MAKE) -C seabios \
-   EXTRAVERSION=$(SEABIOS_EXTRAVERSION) \
+   EXTRAVERSION=$(FIRMWARE_EXTRAVERSION) \
CROSS_PREFIX=$(x86_64_cross_prefix) \
KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \
OUT=$(CURDIR)/seabios/builds/$*/ all
@@ -159,7 +159,7 @@ edk2-version: edk2
 
 efi: edk2-version
$(PYTHON) edk2-build.py --config edk2-build.config \
-   --version-override "$(EDK2_STABLE)-for-qemu" \
+   --version-override "$(EDK2_STABLE)$(FIRMWARE_EXTRAVERSION)" \
--release-date "$(EDK2_DATE)" \
--silent --no-logs
rm -f ../pc-bios/edk2-*.fd.bz2
-- 
2.44.0




[PATCH v2 1/3] edk2: get version + date from git submodule

2024-03-27 Thread Gerd Hoffmann
Turned out hard-coding version and date in the Makefile wasn't a bright
idea.  Updating it on edk2 updates is easily forgotten.  Fetch the info
from git instead.  Store in edk2-version, so this can be committed to
the repo and is present in tarballs too.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index edc234a0e886..783a5cab4f4c 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -52,6 +52,8 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
 #
 EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
 
+-include edk2-version
+
 default help:
@echo "nothing is build by default"
@echo "available build targets:"
@@ -147,10 +149,19 @@ skiboot:
$(MAKE) -C skiboot CROSS=$(powerpc64_cross_prefix)
cp skiboot/skiboot.lid ../pc-bios/skiboot.lid
 
-efi:
+edk2-version: edk2
+   if test -e edk2/.git; then \
+   echo "EDK2_STABLE = $$(cd edk2; git describe --tags --match 
'edk2-stable*')" > $@; \
+   echo "EDK2_DATE = $$(cd edk2; git log -1 --pretty='format:%cd' 
--date='format:%m/%d/%Y')" >> $@; \
+   else \
+   touch $@; \
+   fi
+
+efi: edk2-version
$(PYTHON) edk2-build.py --config edk2-build.config \
-   --version-override "edk2-stable202302-for-qemu" \
-   --release-date "03/01/2023"
+   --version-override "$(EDK2_STABLE)-for-qemu" \
+   --release-date "$(EDK2_DATE)" \
+   --silent --no-logs
rm -f ../pc-bios/edk2-*.fd.bz2
bzip2 --verbose ../pc-bios/edk2-*.fd
 
-- 
2.44.0




[PATCH v2 0/3] edk2: get version + date from git submodule

2024-03-27 Thread Gerd Hoffmann
v2 changes:
 - store version information in git

Gerd Hoffmann (3):
  edk2: get version + date from git submodule
  edk2: commit version info
  edk2/seabios: use common extra version

 roms/Makefile | 25 ++---
 roms/edk2-version |  2 ++
 2 files changed, 20 insertions(+), 7 deletions(-)
 create mode 100644 roms/edk2-version

-- 
2.44.0




Re: [PATCH for-9.0] docs/about: Mark the iaspc machine type as deprecated

2024-03-27 Thread Gerd Hoffmann
On Tue, Mar 26, 2024 at 01:30:48PM +, Mark Cave-Ayland wrote:
> Heh I've actually been using isapc over the past couple of weeks to fire up
> some old programs in a Windows 3 VM :)

I'm wondering why these use cases can't simply use the 'pc' machine
type?

The early pci chipsets of the 90-ies have been designed in a
backward-compatible manner, with devices such as the IDE controller
being mapped to the standard ISA ioports.  So even an historic OS which
does not know what PCI is can run on that hardware, by simply talking to
devices using the standard ISA io ports ...

take care,
  Gerd




Re: [PATCH] edk2: get version + date from git submodule

2024-03-26 Thread Gerd Hoffmann
On Mon, Mar 25, 2024 at 02:55:11PM +, Peter Maydell wrote:
> On Mon, 25 Mar 2024 at 14:45, Gerd Hoffmann  wrote:
> >
> > Turned out hard-coding version and date in the Makefile wasn't a bright
> > idea.  Updating it on edk2 updates is easily forgotten.  Fetch the info
> > from git instead.
> >
> > Signed-off-by: Gerd Hoffmann 
> > ---
> >  roms/Makefile | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/roms/Makefile b/roms/Makefile
> > index edc234a0e886..534eba17ebd0 100644
> > --- a/roms/Makefile
> > +++ b/roms/Makefile
> > @@ -51,6 +51,8 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
> >  # efi ia32, efi x64) into a single rom binary.
> >  #
> >  EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
> > +EDK2_STABLE = $(shell cd edk2; git describe --tags --match 'edk2-stable*')
> > +EDK2_DATE = $(shell cd edk2; git show --pretty='format:%cd' 
> > --date='format:%m/%d/%Y'| head -1)
> 
> I don't think there's any guarantee that the user has 'git'
> installed. scripts/qemu-version avoids using "git describe"
> unless it's building in a git tree.

Hmm.  Have to figure something else then I guess.

> You can avoid the "| head -1" by using
>   git log -1 --pretty='format:%cd' --date='format:%m/%d/%Y'
> I think.

Works.  Thanks.

> Also, does EDK2 really want month/day/year? Typically silly
> choice if so :-)

Yes.

It's the smbios spec being silly btw, this lands more or less
directly in /sys/class/dmi/id/bios_date.  edk2 itself doesn't
care.

take care,
  Gerd




[PATCH v5 0/2] kvm: add support for guest physical bits

2024-03-25 Thread Gerd Hoffmann
The matching kernel bits are here:
https://lore.kernel.org/kvm/20240313125844.912415-1-kra...@redhat.com/T/

ovmf test patches are here:
https://github.com/kraxel/edk2/commits/devel/guest-phys-bits/

Gerd Hoffmann (2):
  kvm: add support for guest physical bits
  target/i386: add guest-phys-bits cpu property

 target/i386/cpu.h |  1 +
 target/i386/cpu.c | 14 ++
 target/i386/kvm/kvm-cpu.c | 31 ++-
 3 files changed, 45 insertions(+), 1 deletion(-)

-- 
2.44.0




[PATCH v5 2/2] target/i386: add guest-phys-bits cpu property

2024-03-25 Thread Gerd Hoffmann
Allows to set guest-phys-bits (cpuid leaf 8008, eax[23:16])
via -cpu $model,guest-phys-bits=$nr.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3b7bd506baf1..79bea83b7b1c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7380,6 +7380,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 if (cpu->phys_bits == 0) {
 cpu->phys_bits = TCG_PHYS_ADDR_BITS;
 }
+if (cpu->guest_phys_bits &&
+(cpu->guest_phys_bits > cpu->phys_bits ||
+cpu->guest_phys_bits < 32)) {
+error_setg(errp, "guest-phys-bits should be between 32 and %u "
+ " (but is %u)",
+ cpu->phys_bits, cpu->guest_phys_bits);
+return;
+}
 } else {
 /* For 32 bit systems don't use the user set value, but keep
  * phys_bits consistent with what we tell the guest.
@@ -7388,6 +7396,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_setg(errp, "phys-bits is not user-configurable in 32 bit");
 return;
 }
+if (cpu->guest_phys_bits != 0) {
+error_setg(errp, "guest-phys-bits is not user-configurable in 32 
bit");
+return;
+}
 
 if (env->features[FEAT_1_EDX] & (CPUID_PSE36 | CPUID_PAE)) {
 cpu->phys_bits = 36;
@@ -7888,6 +7900,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
 DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
+DEFINE_PROP_UINT32("guest-phys-bits", X86CPU, guest_phys_bits, 0),
 DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
 DEFINE_PROP_UINT8("host-phys-bits-limit", X86CPU, host_phys_bits_limit, 0),
 DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
-- 
2.44.0




[PATCH v5 1/2] kvm: add support for guest physical bits

2024-03-25 Thread Gerd Hoffmann
Query kvm for supported guest physical address bits, in cpuid
function 8008, eax[23:16].  Usually this is identical to host
physical address bits.  With NPT or EPT being used this might be
restricted to 48 (max 4-level paging address space size) even if
the host cpu supports more physical address bits.

When set pass this to the guest, using cpuid too.  Guest firmware
can use this to figure how big the usable guest physical address
space is, so PCI bar mapping are actually reachable.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm/kvm-cpu.c | 31 ++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 6b0573807918..83e473584517 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2026,6 +2026,7 @@ struct ArchCPU {
 
 /* Number of physical address bits supported */
 uint32_t phys_bits;
+uint32_t guest_phys_bits;
 
 /* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 33760a2ee163..3b7bd506baf1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6570,6 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
 /* 64 bit processor */
  *eax |= (cpu_x86_virtual_addr_width(env) << 8);
+ *eax |= (cpu->guest_phys_bits << 16);
 }
 *ebx = env->features[FEAT_8000_0008_EBX];
 if (cs->nr_cores * cs->nr_threads > 1) {
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 9c791b7b0520..c5c24f6a8282 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -18,10 +18,33 @@
 #include "kvm_i386.h"
 #include "hw/core/accel-cpu.h"
 
+static void kvm_set_guest_phys_bits(CPUState *cs)
+{
+X86CPU *cpu = X86_CPU(cs);
+uint32_t eax, guest_phys_bits;
+
+eax = kvm_arch_get_supported_cpuid(cs->kvm_state, 0x8008, 0, R_EAX);
+guest_phys_bits = (eax >> 16) & 0xff;
+if (!guest_phys_bits) {
+return;
+}
+
+if (cpu->guest_phys_bits == 0 ||
+cpu->guest_phys_bits > guest_phys_bits) {
+cpu->guest_phys_bits = guest_phys_bits;
+}
+
+if (cpu->host_phys_bits && cpu->host_phys_bits_limit &&
+cpu->guest_phys_bits > cpu->host_phys_bits_limit) {
+cpu->guest_phys_bits = cpu->host_phys_bits_limit;
+}
+}
+
 static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
 {
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = >env;
+bool ret;
 
 /*
  * The realize order is important, since x86_cpu_realize() checks if
@@ -50,7 +73,13 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
MSR_IA32_UCODE_REV);
 }
 }
-return host_cpu_realizefn(cs, errp);
+ret = host_cpu_realizefn(cs, errp);
+if (!ret) {
+return ret;
+}
+
+kvm_set_guest_phys_bits(cs);
+return true;
 }
 
 static bool lmce_supported(void)
-- 
2.44.0




[PATCH] edk2: get version + date from git submodule

2024-03-25 Thread Gerd Hoffmann
Turned out hard-coding version and date in the Makefile wasn't a bright
idea.  Updating it on edk2 updates is easily forgotten.  Fetch the info
from git instead.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index edc234a0e886..534eba17ebd0 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -51,6 +51,8 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
 # efi ia32, efi x64) into a single rom binary.
 #
 EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
+EDK2_STABLE = $(shell cd edk2; git describe --tags --match 'edk2-stable*')
+EDK2_DATE = $(shell cd edk2; git show --pretty='format:%cd' 
--date='format:%m/%d/%Y'| head -1)
 
 default help:
@echo "nothing is build by default"
@@ -149,8 +151,9 @@ skiboot:
 
 efi:
$(PYTHON) edk2-build.py --config edk2-build.config \
-   --version-override "edk2-stable202302-for-qemu" \
-   --release-date "03/01/2023"
+   --version-override "$(EDK2_STABLE)-for-qemu" \
+   --release-date "$(EDK2_DATE)" \
+   --silent --no-logs
rm -f ../pc-bios/edk2-*.fd.bz2
bzip2 --verbose ../pc-bios/edk2-*.fd
 
-- 
2.44.0




Re: [PATCH v4 1/2] kvm: add support for guest physical bits

2024-03-22 Thread Gerd Hoffmann
> > +if (cpu->host_phys_bits_limit &&
> > +cpu->guest_phys_bits > cpu->host_phys_bits_limit) {
> > +cpu->guest_phys_bits = cpu->host_phys_bits_limit;
> 
> host_phys_bits_limit takes effect only when cpu->host_phys_bits is set.
> 
> If users pass configuration like "-cpu
> qemu64,phys-bits=52,host-phys-bits-limit=45", the cpu->guest_phys_bits will
> be set to 45. I think this is not what we want, though the usage seems
> insane.
> 
> We can guard it as
> 
>  if (cpu->host_phys_bits && cpu->host_phys_bits_limit &&
>  cpu->guest_phys_bits > cpu->host_phys_bits_limt)
> {
> }

Yes, makes sense.

> Simpler, we can guard with cpu->phys_bits like below, because
> cpu->host_phys_bits_limit is used to guard cpu->phys_bits in
> host_cpu_realizefn()
> 
>  if (cpu->guest_phys_bits > cpu->phys_bits) {
>   cpu->guest_phys_bits = cpu->phys_bits;
> }

I think I prefer the first version.  The logic is already difficult
enough to follow because it is spread across a bunch of files due to
the different cases we have to handle (tcg, kvm-with-host_phys_bits,
kvm-without-host_phys_bits).

It's not in any way performance-critical, so I happily trade some extra
checks for code which is easier to read.

take care,
  Gerd




[PULL 3/5] roms/efi: exclude efi shell from secure boot builds

2024-03-20 Thread Gerd Hoffmann
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=4641
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Gerd Hoffmann 
Message-ID: <20240314115307.628118-4-kra...@redhat.com>
---
 roms/edk2-build.config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index 05cbafef70cb..ef3eb7beebe7 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -18,6 +18,7 @@ CAVIUM_ERRATUM_27456 = TRUE
 [opts.ovmf.sb.smm]
 SECURE_BOOT_ENABLE   = TRUE
 SMM_REQUIRE  = TRUE
+BUILD_SHELL  = FALSE
 
 [opts.armvirt.silent]
 DEBUG_PRINT_ERROR_LEVEL  = 0x8000
-- 
2.44.0




[PULL 4/5] roms/efi: use pure 64-bit build for edk2-x86_64-secure-code.fd

2024-03-20 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
Message-ID: <20240314115307.628118-5-kra...@redhat.com>
---
 roms/edk2-build.config | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index ef3eb7beebe7..cc9b21154205 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -70,11 +70,11 @@ cpy1 = FV/OVMF_CODE.fd edk2-x86_64-code.fd
 
 [build.ovmf.x86_64.secure]
 desc = ovmf build (64-bit, secure boot)
-conf = OvmfPkg/OvmfPkgIa32X64.dsc
-arch = IA32 X64
+conf = OvmfPkg/OvmfPkgX64.dsc
+arch = X64
 opts = common
ovmf.sb.smm
-plat = Ovmf3264
+plat = OvmfX64
 dest = ../pc-bios
 cpy1 = FV/OVMF_CODE.fd edk2-x86_64-secure-code.fd
 
-- 
2.44.0




[PULL 1/5] roms/efi: clean up edk2 build config

2024-03-20 Thread Gerd Hoffmann
Needed to avoid stale toolchain configurations breaking firmware builds.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Gerd Hoffmann 
Message-ID: <20240314115307.628118-2-kra...@redhat.com>
---
 roms/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/roms/Makefile b/roms/Makefile
index 8e5d8d26a9a0..edc234a0e886 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -187,6 +187,7 @@ clean:
rm -rf seabios/.config seabios/out seabios/builds
$(MAKE) -C ipxe/src veryclean
$(MAKE) -C edk2/BaseTools clean
+   rm -rf 
edk2/Conf/{.cache,BuildEnv.sh,build_rule.txt,target.txt,tools_def.txt}
$(MAKE) -C SLOF clean
rm -rf u-boot/build-e500
$(MAKE) -C u-boot-sam460ex distclean
-- 
2.44.0




[PULL 2/5] roms/efi: drop workaround for edk2-stable202308

2024-03-20 Thread Gerd Hoffmann
Not needed for newer edk2 versions.

Signed-off-by: Gerd Hoffmann 
Message-ID: <20240314115307.628118-3-kra...@redhat.com>
---
 roms/edk2-build.config | 6 --
 1 file changed, 6 deletions(-)

diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index 0d367dbdb775..05cbafef70cb 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -32,9 +32,6 @@ PcdDxeNxMemoryProtectionPolicy = 0xC0007FD1
 # shim.efi has broken MemAttr code
 PcdUninstallMemAttrProtocol= TRUE
 
-[pcds.workaround.202308]
-PcdFirstTimeWakeUpAPsBySipi = FALSE
-
 

 # i386
 
@@ -66,7 +63,6 @@ desc = ovmf build (64-bit)
 conf = OvmfPkg/OvmfPkgX64.dsc
 arch = X64
 opts = common
-pcds = workaround.202308
 plat = OvmfX64
 dest = ../pc-bios
 cpy1 = FV/OVMF_CODE.fd edk2-x86_64-code.fd
@@ -77,7 +73,6 @@ conf = OvmfPkg/OvmfPkgIa32X64.dsc
 arch = IA32 X64
 opts = common
ovmf.sb.smm
-pcds = workaround.202308
 plat = Ovmf3264
 dest = ../pc-bios
 cpy1 = FV/OVMF_CODE.fd edk2-x86_64-secure-code.fd
@@ -87,7 +82,6 @@ desc = ovmf build for microvm
 conf = OvmfPkg/Microvm/MicrovmX64.dsc
 arch = X64
 opts = common
-pcds = workaround.202308
 plat = MicrovmX64
 dest = ../pc-bios
 cpy1 = FV/MICROVM.fd  edk2-x86_64-microvm.fd
-- 
2.44.0




[PULL 0/5] Edk2 20240320 patches

2024-03-20 Thread Gerd Hoffmann
The following changes since commit ba49d760eb04630e7b15f423ebecf6c871b8f77b:

  Merge tag 'pull-maintainer-final-130324-1' of https://gitlab.com/stsquad/qemu 
into staging (2024-03-13 15:12:14 +)

are available in the Git repository at:

  https://gitlab.com/kraxel/qemu.git tags/edk2-20240320-pull-request

for you to fetch changes up to 4a1babe58a1b3cd2c493ee6e0d774e70f62ad9c3:

  update edk2 binaries for arm, risc-v and x86 secure boot. (2024-03-19 
16:42:10 +0100)


edk2: cleanup fix, update build config, rebuild binaries.



Gerd Hoffmann (5):
  roms/efi: clean up edk2 build config
  roms/efi: drop workaround for edk2-stable202308
  roms/efi: exclude efi shell from secure boot builds
  roms/efi: use pure 64-bit build for edk2-x86_64-secure-code.fd
  update edk2 binaries for arm, risc-v and x86 secure boot.

 pc-bios/edk2-aarch64-code.fd.bz2 | Bin 1589320 -> 1589310 bytes
 pc-bios/edk2-arm-code.fd.bz2 | Bin 1571418 -> 1571693 bytes
 pc-bios/edk2-i386-secure-code.fd.bz2 | Bin 2130741 -> 1876986 bytes
 pc-bios/edk2-riscv-code.fd.bz2   | Bin 1345420 -> 1289160 bytes
 roms/Makefile|   1 +
 roms/edk2-build.config   |  13 -
 6 files changed, 5 insertions(+), 9 deletions(-)

-- 
2.44.0




[PATCH v4 2/2] target/i386: add guest-phys-bits cpu property

2024-03-18 Thread Gerd Hoffmann
Allows to set guest-phys-bits (cpuid leaf 8008, eax[23:16])
via -cpu $model,guest-phys-bits=$nr.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c88c895a5b3e..e0d73b6ec654 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7380,6 +7380,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 if (cpu->phys_bits == 0) {
 cpu->phys_bits = TCG_PHYS_ADDR_BITS;
 }
+if (cpu->guest_phys_bits &&
+(cpu->guest_phys_bits > cpu->phys_bits ||
+cpu->guest_phys_bits < 32)) {
+error_setg(errp, "guest-phys-bits should be between 32 and %u "
+ " (but is %u)",
+ cpu->phys_bits, cpu->guest_phys_bits);
+return;
+}
 } else {
 /* For 32 bit systems don't use the user set value, but keep
  * phys_bits consistent with what we tell the guest.
@@ -7388,6 +7396,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_setg(errp, "phys-bits is not user-configurable in 32 bit");
 return;
 }
+if (cpu->guest_phys_bits != 0) {
+error_setg(errp, "guest-phys-bits is not user-configurable in 32 
bit");
+return;
+}
 
 if (env->features[FEAT_1_EDX] & (CPUID_PSE36 | CPUID_PAE)) {
 cpu->phys_bits = 36;
@@ -7888,6 +7900,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
 DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
+DEFINE_PROP_UINT32("guest-phys-bits", X86CPU, guest_phys_bits, 0),
 DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
 DEFINE_PROP_UINT8("host-phys-bits-limit", X86CPU, host_phys_bits_limit, 0),
 DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
-- 
2.44.0




[PATCH v4 1/2] kvm: add support for guest physical bits

2024-03-18 Thread Gerd Hoffmann
Query kvm for supported guest physical address bits, in cpuid
function 8008, eax[23:16].  Usually this is identical to host
physical address bits.  With NPT or EPT being used this might be
restricted to 48 (max 4-level paging address space size) even if
the host cpu supports more physical address bits.

When set pass this to the guest, using cpuid too.  Guest firmware
can use this to figure how big the usable guest physical address
space is, so PCI bar mapping are actually reachable.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm/kvm-cpu.c | 31 ++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 952174bb6f52..d427218827f6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2026,6 +2026,7 @@ struct ArchCPU {
 
 /* Number of physical address bits supported */
 uint32_t phys_bits;
+uint32_t guest_phys_bits;
 
 /* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9a210d8d9290..c88c895a5b3e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6570,6 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
 /* 64 bit processor */
  *eax |= (cpu_x86_virtual_addr_width(env) << 8);
+ *eax |= (cpu->guest_phys_bits << 16);
 }
 *ebx = env->features[FEAT_8000_0008_EBX];
 if (cs->nr_cores * cs->nr_threads > 1) {
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 9c791b7b0520..5132bb96abd5 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -18,10 +18,33 @@
 #include "kvm_i386.h"
 #include "hw/core/accel-cpu.h"
 
+static void kvm_set_guest_phys_bits(CPUState *cs)
+{
+X86CPU *cpu = X86_CPU(cs);
+uint32_t eax, guest_phys_bits;
+
+eax = kvm_arch_get_supported_cpuid(cs->kvm_state, 0x8008, 0, R_EAX);
+guest_phys_bits = (eax >> 16) & 0xff;
+if (!guest_phys_bits) {
+return;
+}
+
+if (cpu->guest_phys_bits == 0 ||
+cpu->guest_phys_bits > guest_phys_bits) {
+cpu->guest_phys_bits = guest_phys_bits;
+}
+
+if (cpu->host_phys_bits_limit &&
+cpu->guest_phys_bits > cpu->host_phys_bits_limit) {
+cpu->guest_phys_bits = cpu->host_phys_bits_limit;
+}
+}
+
 static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
 {
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = >env;
+bool ret;
 
 /*
  * The realize order is important, since x86_cpu_realize() checks if
@@ -50,7 +73,13 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
MSR_IA32_UCODE_REV);
 }
 }
-return host_cpu_realizefn(cs, errp);
+ret = host_cpu_realizefn(cs, errp);
+if (!ret) {
+return ret;
+}
+
+kvm_set_guest_phys_bits(cs);
+return true;
 }
 
 static bool lmce_supported(void)
-- 
2.44.0




[PATCH v4 0/2] kvm: add support for guest physical bits

2024-03-18 Thread Gerd Hoffmann
The matching kernel bits are here:
https://lore.kernel.org/kvm/20240313125844.912415-1-kra...@redhat.com/T/

ovmf test patches are here:
https://github.com/kraxel/edk2/commits/devel/guest-phys-bits/

Gerd Hoffmann (2):
  kvm: add support for guest physical bits
  target/i386: add guest-phys-bits cpu property

 target/i386/cpu.h |  1 +
 target/i386/cpu.c | 14 ++
 target/i386/kvm/kvm-cpu.c | 31 ++-
 3 files changed, 45 insertions(+), 1 deletion(-)

-- 
2.44.0




Re: [PATCH v3 2/3] kvm: add support for guest physical bits

2024-03-18 Thread Gerd Hoffmann
  Hi,

> > diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> > index 9c791b7b0520..a2b7bfaeadf8 100644
> > --- a/target/i386/kvm/kvm-cpu.c
> > +++ b/target/i386/kvm/kvm-cpu.c
> > @@ -18,10 +18,36 @@
> >   #include "kvm_i386.h"
> >   #include "hw/core/accel-cpu.h"
> > +static void kvm_set_guest_phys_bits(CPUState *cs)
> > +{
> > +X86CPU *cpu = X86_CPU(cs);
> > +uint32_t eax, guest_phys_bits;
> > +
> > +if (!cpu->host_phys_bits) {
> > +return;
> > +}
> 
> This needs explanation of why. What if users set the phys-bits to exactly
> host's value, via "-cpu xxx,phys-bits=host's value"?

If host_phys_bits is not enabled it is possible to set phys-bits to any
value today (including invalid values not supported by the host).  With
this the same applies to guest_phys_bits.

My intention was to continue allowing any guest_phys_bits + phys_bits
with TCG, for testing purposes.  But thinking again this logic is
flawed, if TCG is used the control flow doesn't land here in the first
place.

So, I think this can be dropped.

> > +ret = host_cpu_realizefn(cs, errp);
> 
> We need to check ret and return if !ret;

Fixed.

thanks,
  Gerd




Re: [PATCH v3 2/3] kvm: add support for guest physical bits

2024-03-18 Thread Gerd Hoffmann
  Hi,

> > +if (cpu->guest_phys_bits > cpu->host_phys_bits_limit) {
> > +cpu->guest_phys_bits = cpu->host_phys_bits_limit;
> 
> host_phys_bits_limit is zero by default, so I think it is better to be
> like:
> 
> if (cpu->host_phys_bits_limit &&
> cpu->guest_phys_bits > cpu->host_phys_bits_limit) {
> cpu->guest_phys_bits = cpu->host_phys_bits_limit;
> }

Good point, fixed.

thanks,
  Gerd




Re: [PATCH 03/12] uefi-test-tools: Add support for python based build script

2024-03-15 Thread Gerd Hoffmann
> +Build/bios-tables-test.%.efi:
> + $(PYTHON) ../../roms/edk2-build.py --config uefi-test-build.config

Adding '--match $*' will build one arch instead of all.




Re: [PATCH 02/12] uefi-test-tools/UefiTestToolsPkg: Add RISC-V support

2024-03-15 Thread Gerd Hoffmann
On Fri, Mar 15, 2024 at 06:35:09PM +0530, Sunil V L wrote:
> Enable building the test application for RISC-V with appropriate
> dependencies updated.
> 
> Signed-off-by: Sunil V L 
> ---
>  tests/uefi-test-tools/UefiTestToolsPkg/UefiTestToolsPkg.dsc | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Acked-by: Gerd Hoffmann 




Re: [PATCH 01/12] roms/edk2-build.py: Add --module support

2024-03-15 Thread Gerd Hoffmann
On Fri, Mar 15, 2024 at 06:35:08PM +0530, Sunil V L wrote:
> UefiTestToolsPkg which should use edk2-build.py needs --module parameter
> support. Add this optional parameter handling.

I don't think this is needed.  By default everything listed in
[Components] should be built, which is just that one module we
have ;)

take care,
  Gerd




[PATCH 2/5] roms/efi: drop workaround for edk2-stable202308

2024-03-14 Thread Gerd Hoffmann
Not needed for newer edk2 versions.

Signed-off-by: Gerd Hoffmann 
---
 roms/edk2-build.config | 6 --
 1 file changed, 6 deletions(-)

diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index 0d367dbdb775..05cbafef70cb 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -32,9 +32,6 @@ PcdDxeNxMemoryProtectionPolicy = 0xC0007FD1
 # shim.efi has broken MemAttr code
 PcdUninstallMemAttrProtocol= TRUE
 
-[pcds.workaround.202308]
-PcdFirstTimeWakeUpAPsBySipi = FALSE
-
 

 # i386
 
@@ -66,7 +63,6 @@ desc = ovmf build (64-bit)
 conf = OvmfPkg/OvmfPkgX64.dsc
 arch = X64
 opts = common
-pcds = workaround.202308
 plat = OvmfX64
 dest = ../pc-bios
 cpy1 = FV/OVMF_CODE.fd edk2-x86_64-code.fd
@@ -77,7 +73,6 @@ conf = OvmfPkg/OvmfPkgIa32X64.dsc
 arch = IA32 X64
 opts = common
ovmf.sb.smm
-pcds = workaround.202308
 plat = Ovmf3264
 dest = ../pc-bios
 cpy1 = FV/OVMF_CODE.fd edk2-x86_64-secure-code.fd
@@ -87,7 +82,6 @@ desc = ovmf build for microvm
 conf = OvmfPkg/Microvm/MicrovmX64.dsc
 arch = X64
 opts = common
-pcds = workaround.202308
 plat = MicrovmX64
 dest = ../pc-bios
 cpy1 = FV/MICROVM.fd  edk2-x86_64-microvm.fd
-- 
2.44.0




[PATCH 0/5] roms/efi: cleanup fix, config update, ekd2 binary rebuild

2024-03-14 Thread Gerd Hoffmann



Gerd Hoffmann (5):
  roms/efi: clean up edk2 build config
  roms/efi: drop workaround for edk2-stable202308
  roms/efi: exclude efi shell from secure boot builds
  roms/efi: use pure 64-bit build for edk2-x86_64-secure-code.fd
  update edk2 binaries for arm, risc-v and x86 secure boot.

 pc-bios/edk2-aarch64-code.fd.bz2   | Bin 1589320 -> 1589310 bytes
 pc-bios/edk2-arm-code.fd.bz2   | Bin 1571418 -> 1571693 bytes
 pc-bios/edk2-i386-secure-code.fd.bz2   | Bin 2130741 -> 1876986 bytes
 pc-bios/edk2-riscv-code.fd.bz2 | Bin 1345420 -> 1289160 bytes
 pc-bios/edk2-x86_64-secure-code.fd.bz2 | Bin 2214892 -> 1967967 bytes
 roms/Makefile  |   1 +
 roms/edk2-build.config |  13 -
 7 files changed, 5 insertions(+), 9 deletions(-)

-- 
2.44.0




[PATCH 3/5] roms/efi: exclude efi shell from secure boot builds

2024-03-14 Thread Gerd Hoffmann
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=4641
Signed-off-by: Gerd Hoffmann 
---
 roms/edk2-build.config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index 05cbafef70cb..ef3eb7beebe7 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -18,6 +18,7 @@ CAVIUM_ERRATUM_27456 = TRUE
 [opts.ovmf.sb.smm]
 SECURE_BOOT_ENABLE   = TRUE
 SMM_REQUIRE  = TRUE
+BUILD_SHELL  = FALSE
 
 [opts.armvirt.silent]
 DEBUG_PRINT_ERROR_LEVEL  = 0x8000
-- 
2.44.0




[PATCH 4/5] roms/efi: use pure 64-bit build for edk2-x86_64-secure-code.fd

2024-03-14 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 roms/edk2-build.config | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/roms/edk2-build.config b/roms/edk2-build.config
index ef3eb7beebe7..cc9b21154205 100644
--- a/roms/edk2-build.config
+++ b/roms/edk2-build.config
@@ -70,11 +70,11 @@ cpy1 = FV/OVMF_CODE.fd edk2-x86_64-code.fd
 
 [build.ovmf.x86_64.secure]
 desc = ovmf build (64-bit, secure boot)
-conf = OvmfPkg/OvmfPkgIa32X64.dsc
-arch = IA32 X64
+conf = OvmfPkg/OvmfPkgX64.dsc
+arch = X64
 opts = common
ovmf.sb.smm
-plat = Ovmf3264
+plat = OvmfX64
 dest = ../pc-bios
 cpy1 = FV/OVMF_CODE.fd edk2-x86_64-secure-code.fd
 
-- 
2.44.0




[PATCH 1/5] roms/efi: clean up edk2 build config

2024-03-14 Thread Gerd Hoffmann
Needed to avoid stale toolchain configurations breaking firmware builds.

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/roms/Makefile b/roms/Makefile
index 8e5d8d26a9a0..edc234a0e886 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -187,6 +187,7 @@ clean:
rm -rf seabios/.config seabios/out seabios/builds
$(MAKE) -C ipxe/src veryclean
$(MAKE) -C edk2/BaseTools clean
+   rm -rf 
edk2/Conf/{.cache,BuildEnv.sh,build_rule.txt,target.txt,tools_def.txt}
$(MAKE) -C SLOF clean
rm -rf u-boot/build-e500
$(MAKE) -C u-boot-sam460ex distclean
-- 
2.44.0




[PATCH v3 2/3] kvm: add support for guest physical bits

2024-03-13 Thread Gerd Hoffmann
Query kvm for supported guest physical address bits, in cpuid
function 8008, eax[23:16].  Usually this is identical to host
physical address bits.  With NPT or EPT being used this might be
restricted to 48 (max 4-level paging address space size) even if
the host cpu supports more physical address bits.

When set pass this to the guest, using cpuid too.  Guest firmware
can use this to figure how big the usable guest physical address
space is, so PCI bar mapping are actually reachable.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm/kvm-cpu.c | 32 +++-
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 952174bb6f52..d427218827f6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2026,6 +2026,7 @@ struct ArchCPU {
 
 /* Number of physical address bits supported */
 uint32_t phys_bits;
+uint32_t guest_phys_bits;
 
 /* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9a210d8d9290..c88c895a5b3e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6570,6 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
 /* 64 bit processor */
  *eax |= (cpu_x86_virtual_addr_width(env) << 8);
+ *eax |= (cpu->guest_phys_bits << 16);
 }
 *ebx = env->features[FEAT_8000_0008_EBX];
 if (cs->nr_cores * cs->nr_threads > 1) {
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 9c791b7b0520..a2b7bfaeadf8 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -18,10 +18,36 @@
 #include "kvm_i386.h"
 #include "hw/core/accel-cpu.h"
 
+static void kvm_set_guest_phys_bits(CPUState *cs)
+{
+X86CPU *cpu = X86_CPU(cs);
+uint32_t eax, guest_phys_bits;
+
+if (!cpu->host_phys_bits) {
+return;
+}
+
+eax = kvm_arch_get_supported_cpuid(cs->kvm_state, 0x8008, 0, R_EAX);
+guest_phys_bits = (eax >> 16) & 0xff;
+if (!guest_phys_bits) {
+return;
+}
+
+if (cpu->guest_phys_bits == 0 ||
+cpu->guest_phys_bits > guest_phys_bits) {
+cpu->guest_phys_bits = guest_phys_bits;
+}
+
+if (cpu->guest_phys_bits > cpu->host_phys_bits_limit) {
+cpu->guest_phys_bits = cpu->host_phys_bits_limit;
+}
+}
+
 static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
 {
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = >env;
+bool ret;
 
 /*
  * The realize order is important, since x86_cpu_realize() checks if
@@ -50,7 +76,11 @@ static bool kvm_cpu_realizefn(CPUState *cs, Error **errp)
MSR_IA32_UCODE_REV);
 }
 }
-return host_cpu_realizefn(cs, errp);
+ret = host_cpu_realizefn(cs, errp);
+
+kvm_set_guest_phys_bits(cs);
+
+return ret;
 }
 
 static bool lmce_supported(void)
-- 
2.44.0




[PATCH v3 1/3] [debug] log kvm supported cpuid

2024-03-13 Thread Gerd Hoffmann
---
 target/i386/kvm/kvm.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index e68cbe929302..2f5e3b9febf9 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -276,6 +276,20 @@ static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int 
max)
 exit(1);
 }
 }
+
+{
+int i;
+
+for (i = 0; i < cpuid->nent; i++) {
+fprintf(stderr, "cpuid: %8x/%d - %8x %8x %8x %8x\n",
+cpuid->entries[i].function,
+cpuid->entries[i].index,
+cpuid->entries[i].eax,
+cpuid->entries[i].ebx,
+cpuid->entries[i].ecx,
+cpuid->entries[i].edx);
+}
+}
 return cpuid;
 }
 
-- 
2.44.0




[PATCH v3 3/3] target/i386: add guest-phys-bits cpu property

2024-03-13 Thread Gerd Hoffmann
Allows to set guest-phys-bits (cpuid leaf 8008, eax[23:16])
via -cpu $model,guest-phys-bits=$nr.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index c88c895a5b3e..e0d73b6ec654 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7380,6 +7380,14 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 if (cpu->phys_bits == 0) {
 cpu->phys_bits = TCG_PHYS_ADDR_BITS;
 }
+if (cpu->guest_phys_bits &&
+(cpu->guest_phys_bits > cpu->phys_bits ||
+cpu->guest_phys_bits < 32)) {
+error_setg(errp, "guest-phys-bits should be between 32 and %u "
+ " (but is %u)",
+ cpu->phys_bits, cpu->guest_phys_bits);
+return;
+}
 } else {
 /* For 32 bit systems don't use the user set value, but keep
  * phys_bits consistent with what we tell the guest.
@@ -7388,6 +7396,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_setg(errp, "phys-bits is not user-configurable in 32 bit");
 return;
 }
+if (cpu->guest_phys_bits != 0) {
+error_setg(errp, "guest-phys-bits is not user-configurable in 32 
bit");
+return;
+}
 
 if (env->features[FEAT_1_EDX] & (CPUID_PSE36 | CPUID_PAE)) {
 cpu->phys_bits = 36;
@@ -7888,6 +7900,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("x-force-features", X86CPU, force_features, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
 DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 0),
+DEFINE_PROP_UINT32("guest-phys-bits", X86CPU, guest_phys_bits, 0),
 DEFINE_PROP_BOOL("host-phys-bits", X86CPU, host_phys_bits, false),
 DEFINE_PROP_UINT8("host-phys-bits-limit", X86CPU, host_phys_bits_limit, 0),
 DEFINE_PROP_BOOL("fill-mtrr-mask", X86CPU, fill_mtrr_mask, true),
-- 
2.44.0




[PATCH v3 0/3] kvm: add support for guest physical bits

2024-03-13 Thread Gerd Hoffmann
The matching kernel bits are here:
https://lore.kernel.org/kvm/20240313125844.912415-1-kra...@redhat.com/T/

ovmf test patches are here:
https://github.com/kraxel/edk2/commits/devel/guest-phys-bits/

Gerd Hoffmann (3):
  [debug] log kvm supported cpuid
  kvm: add support for guest physical bits
  target/i386: add guest-phys-bits cpu property

 target/i386/cpu.h |  1 +
 target/i386/cpu.c | 14 ++
 target/i386/kvm/kvm-cpu.c | 32 +++-
 target/i386/kvm/kvm.c | 14 ++
 4 files changed, 60 insertions(+), 1 deletion(-)

-- 
2.44.0




Re: [PATCH v2 2/2] kvm: add support for guest physical bits

2024-03-13 Thread Gerd Hoffmann
On Wed, Mar 13, 2024 at 02:39:32PM +0800, Tao Su wrote:
> On Tue, Mar 05, 2024 at 11:52:33AM +0100, Gerd Hoffmann wrote:
> > Query kvm for supported guest physical address bits, in cpuid
> > function 8008, eax[23:16].  Usually this is identical to host
> > physical address bits.  With NPT or EPT being used this might be
> > restricted to 48 (max 4-level paging address space size) even if
> > the host cpu supports more physical address bits.
> > 
> > When set pass this to the guest, using cpuid too.  Guest firmware
> > can use this to figure how big the usable guest physical address
> > space is, so PCI bar mapping are actually reachable.
> 
> If this patch is applied, do you have plans to implement it in
> OVMF/Seabios?

Yes.  ovmf test patches:

https://github.com/kraxel/edk2/commits/devel/guest-phys-bits/

take care,
  Gerd




Re: [PATCH v2 2/2] kvm: add support for guest physical bits

2024-03-11 Thread Gerd Hoffmann
  Hi,

> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 952174bb6f52..d427218827f6 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > +guest_phys_bits = kvm_get_guest_phys_bits(cs->kvm_state);
> > +if (guest_phys_bits &&
> > +(cpu->guest_phys_bits == 0 ||
> > + cpu->guest_phys_bits > guest_phys_bits)) {
> > +cpu->guest_phys_bits = guest_phys_bits;
> > +}
> 
> Like Xiaoyao mentioned, the right place for this is kvm_cpu_realizefn,
> after host_cpu_realizefn returns. It should also be conditional on
> cpu->host_phys_bits.

Ok.

> It also makes sense to:
> 
> - make kvm_get_guest_phys_bits() return bits 7:0 if bits 23:16 are zero
> 
> - here, set cpu->guest_phys_bits only if it is not equal to
> cpu->phys_bits (this undoes the previous suggestion, but I think it's
> cleaner)

Not sure about that.

I think it would be good to have a backward compatibility story.
Currently neither the kernel nor qemu set guest_phys_bits.  So if the
firmware finds guest_phys_bits == 0 it does not know whenever ...

  (a) kernel or qemu being too old, or
  (b) no restrictions apply, it is safe to go with phys_bits.

One easy option would be to always let qemu pass through guest_phys_bits
from the kernel, even in case it is equal to phys_bits.

> - add a property in x86_cpu_properties[] to allow configuration with TCG.

Was thinking about configuration too.  Not sure it is a good idea to
add yet another phys-bits config option to the mix of options we already
have ...

In case host_phys_bits=true qemu could simply use
min(kernel guest-phys-bits,host-phys-bits-limit)

For the host_phys_bits=false case it would probably be best to just
not set guest_phys_bits.

take care,
  Gerd




Re: [PATCH v2 1/2] [debug] log kvm supported cpuid

2024-03-05 Thread Gerd Hoffmann
>  target/i386/kvm/kvm.c | 14 ++

Oops, that was not meant to be posted.
Please ignore and look at patch 2/2.

thanks,
  Gerd




[PATCH v2 0/2] kvm: add support for guest physical bits

2024-03-05 Thread Gerd Hoffmann
The matching kernel bits are here:
https://lore.kernel.org/kvm/20240305103506.613950-1-kra...@redhat.com/T/

Gerd Hoffmann (2):
  [debug] log kvm supported cpuid
  kvm: add support for guest physical bits

 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm/kvm.c | 31 +++
 3 files changed, 33 insertions(+)

-- 
2.44.0




[PATCH v2 2/2] kvm: add support for guest physical bits

2024-03-05 Thread Gerd Hoffmann
Query kvm for supported guest physical address bits, in cpuid
function 8008, eax[23:16].  Usually this is identical to host
physical address bits.  With NPT or EPT being used this might be
restricted to 48 (max 4-level paging address space size) even if
the host cpu supports more physical address bits.

When set pass this to the guest, using cpuid too.  Guest firmware
can use this to figure how big the usable guest physical address
space is, so PCI bar mapping are actually reachable.

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm/kvm.c | 17 +
 3 files changed, 19 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 952174bb6f52..d427218827f6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2026,6 +2026,7 @@ struct ArchCPU {
 
 /* Number of physical address bits supported */
 uint32_t phys_bits;
+uint32_t guest_phys_bits;
 
 /* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2666ef380891..1a6cfc75951e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6570,6 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
 /* 64 bit processor */
  *eax |= (cpu_x86_virtual_addr_width(env) << 8);
+ *eax |= (cpu->guest_phys_bits << 16);
 }
 *ebx = env->features[FEAT_8000_0008_EBX];
 if (cs->nr_cores * cs->nr_threads > 1) {
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 7298822cb511..ce22dfcaa661 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -238,6 +238,15 @@ static int kvm_get_tsc(CPUState *cs)
 return 0;
 }
 
+/* return cpuid fn 8000_0008 eax[23:16] aka GuestPhysBits */
+static int kvm_get_guest_phys_bits(KVMState *s)
+{
+uint32_t eax;
+
+eax = kvm_arch_get_supported_cpuid(s, 0x8008, 0, R_EAX);
+return (eax >> 16) & 0xff;
+}
+
 static inline void do_kvm_synchronize_tsc(CPUState *cpu, run_on_cpu_data arg)
 {
 kvm_get_tsc(cpu);
@@ -1730,6 +1739,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = >env;
 uint32_t limit, i, j, cpuid_i;
+uint32_t guest_phys_bits;
 uint32_t unused;
 struct kvm_cpuid_entry2 *c;
 uint32_t signature[3];
@@ -1765,6 +1775,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
 env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY;
 
+guest_phys_bits = kvm_get_guest_phys_bits(cs->kvm_state);
+if (guest_phys_bits &&
+(cpu->guest_phys_bits == 0 ||
+ cpu->guest_phys_bits > guest_phys_bits)) {
+cpu->guest_phys_bits = guest_phys_bits;
+}
+
 /*
  * kvm_hyperv_expand_features() is called here for the second time in case
  * KVM_CAP_SYS_HYPERV_CPUID is not supported. While we can't possibly 
handle
-- 
2.44.0




[PATCH v2 1/2] [debug] log kvm supported cpuid

2024-03-05 Thread Gerd Hoffmann
---
 target/i386/kvm/kvm.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 42970ab046fa..7298822cb511 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -276,6 +276,20 @@ static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int 
max)
 exit(1);
 }
 }
+
+{
+int i;
+
+for (i = 0; i < cpuid->nent; i++) {
+fprintf(stderr, "cpuid: %8x/%d - %8x %8x %8x %8x\n",
+cpuid->entries[i].function,
+cpuid->entries[i].index,
+cpuid->entries[i].eax,
+cpuid->entries[i].ebx,
+cpuid->entries[i].ecx,
+cpuid->entries[i].edx);
+}
+}
 return cpuid;
 }
 
-- 
2.44.0




Re: [PATCH 1/1] kvm: add support for guest physical bits

2024-03-04 Thread Gerd Hoffmann
On Mon, Mar 04, 2024 at 09:54:40AM +0800, Xiaoyao Li wrote:
> On 3/1/2024 6:17 PM, Gerd Hoffmann wrote:
> > query kvm for supported guest physical address bits using
> > KVM_CAP_VM_GPA_BITS.  Expose the value to the guest via cpuid
> > (leaf 0x8008, eax, bits 16-23).
> > 
> > Signed-off-by: Gerd Hoffmann 
> > ---
> >   target/i386/cpu.h | 1 +
> >   target/i386/cpu.c | 1 +
> >   target/i386/kvm/kvm.c | 8 
> >   3 files changed, 10 insertions(+)
> > 
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 952174bb6f52..d427218827f6 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -2026,6 +2026,7 @@ struct ArchCPU {
> >   /* Number of physical address bits supported */
> >   uint32_t phys_bits;
> > +uint32_t guest_phys_bits;
> >   /* in order to simplify APIC support, we leave this pointer to the
> >  user */
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 2666ef380891..1a6cfc75951e 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -6570,6 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> > uint32_t count,
> >   if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
> >   /* 64 bit processor */
> >*eax |= (cpu_x86_virtual_addr_width(env) << 8);
> > + *eax |= (cpu->guest_phys_bits << 16);
> 
> I think you misunderstand this field.
> 
> If you expose this field to guest, it's the information for nested guest.
> i.e., the guest itself runs as a hypervisor will know its nested guest can
> have guest_phys_bits for physical addr.

I think those limits (l1 + l2 guest phys-bits) are identical, no?

The problem this tries to solve is that making the guest phys-bits
smaller than the host phys-bits is problematic (which why we have
allow_smaller_maxphyaddr), but nevertheless there are cases where
the usable guest physical address space is smaller than the host
physical address space.  One case is intel processors with phys-bits
larger than 48 and 4-level EPT.  Another case is amd processors with
phys-bits larger than 48 and the l0 hypervisor using 4-level paging.

The guest needs to know that limit, specifically the guest firmware
so it knows where it can map PCI bars.

take care,
  Gerd




[PATCH 1/1] kvm: add support for guest physical bits

2024-03-01 Thread Gerd Hoffmann
query kvm for supported guest physical address bits using
KVM_CAP_VM_GPA_BITS.  Expose the value to the guest via cpuid
(leaf 0x8008, eax, bits 16-23).

Signed-off-by: Gerd Hoffmann 
---
 target/i386/cpu.h | 1 +
 target/i386/cpu.c | 1 +
 target/i386/kvm/kvm.c | 8 
 3 files changed, 10 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 952174bb6f52..d427218827f6 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2026,6 +2026,7 @@ struct ArchCPU {
 
 /* Number of physical address bits supported */
 uint32_t phys_bits;
+uint32_t guest_phys_bits;
 
 /* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2666ef380891..1a6cfc75951e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6570,6 +6570,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) {
 /* 64 bit processor */
  *eax |= (cpu_x86_virtual_addr_width(env) << 8);
+ *eax |= (cpu->guest_phys_bits << 16);
 }
 *ebx = env->features[FEAT_8000_0008_EBX];
 if (cs->nr_cores * cs->nr_threads > 1) {
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 42970ab046fa..e06c9d66bb01 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1716,6 +1716,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = >env;
 uint32_t limit, i, j, cpuid_i;
+uint32_t guest_phys_bits;
 uint32_t unused;
 struct kvm_cpuid_entry2 *c;
 uint32_t signature[3];
@@ -1751,6 +1752,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
 env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY;
 
+guest_phys_bits = kvm_check_extension(cs->kvm_state, KVM_CAP_VM_GPA_BITS);
+if (guest_phys_bits &&
+(cpu->guest_phys_bits == 0 ||
+ cpu->guest_phys_bits > guest_phys_bits)) {
+cpu->guest_phys_bits = guest_phys_bits;
+}
+
 /*
  * kvm_hyperv_expand_features() is called here for the second time in case
  * KVM_CAP_SYS_HYPERV_CPUID is not supported. While we can't possibly 
handle
-- 
2.44.0




[PATCH 0/1] kvm: add support for guest physical bits

2024-03-01 Thread Gerd Hoffmann
The matching kernel bits are here:
https://lore.kernel.org/kvm/20240301101410.356007-1-kra...@redhat.com/T/

Gerd Hoffmann (1):
  kvm: add support for guest physical bits

 target/i386/cpu.h | 1 +
 target/i386/cpu.c | 1 +
 target/i386/kvm/kvm.c | 8 
 3 files changed, 10 insertions(+)

-- 
2.44.0




Re: No virtio devices in SeaBIOS VMs

2024-02-29 Thread Gerd Hoffmann
  Hi,

> UEFI guests seem not to be affected in any way, no matter amount of RAM
> or CPU model (well, of course, since it's a SeaBIOS commit! :-D What I
> mean is that there seems to be nothing in edk2 that induces the same
> behavior).

That used to be a problem with UEFI too.

> A way of working this around (beside switching to UEFI or to cpu=host)
> is to turn on host-phys-bits, e.g., with ' mode="passthrough"/>' in the XML.

Sounds like the phys-bits of your vcpus is larger than the value the
host actually supports.  So if the firmware tries to use the whole
address space available things break.

Both UEFI and SeaBIOS have a similar heuristic to figure whenever they
can trust phys-bits or not, and those checks consider upstream qemu
behavior (use phys-bits=40 for all cpu types except 'host').

When this came up with UEFI the root cause turned out to be that suse
qemu derived from upstream qemu.  There have been phys-bits values other
than 40 which where not valid (i.e. larger than supported by the host).

I don't know how that was solved in the end.  But given that we see
similar problems again with SeaBIOS I suspect it was patched in suse
OVMF not suse qemu.

> It is, however, a bit impractical to have to do this for all the VMs
> that one may have... Especially if they're a lot! :-)

I'd actually recommend to run all VMs with host-phys-bits=on (and use
host-phys-bits-limit=value if you need phys-bits being equal on all
machines of a heterogeneous cluster for live migration compatibility).

phys-bits being too big never was a valid configuration.  It only
happened to work because the firmware was very conservative with address
space usage.  That strategy became increasingly problematic though.
These days GPUs and NPUs can have gigabytes of device memory and equally
large pci memory bars ...

take care,
  Gerd




Re: [PATCH v4] pc: q35: Bump max_cpus to 4096 vcpus

2024-02-28 Thread Gerd Hoffmann
On Wed, Feb 21, 2024 at 07:32:27PM +0530, Ani Sinha wrote:
> Since commit f10a570b093e6 ("KVM: x86: Add CONFIG_KVM_MAX_NR_VCPUS to allow 
> up to 4096 vCPUs")
> Linux kernel can support upto a maximum number of 4096 vCPUS when MAXSMP is
> enabled in the kernel. At present, QEMU has been tested to correctly boot a
> linux guest with 4096 vcpus with edk2 pending various upstream EDK2 fixes
> which will probably be in the 2024-05 release to be released in the coming

Merged meanwhile, so 2024-05 release is a sure thing and latest edk2
master branch is good too.

You might refine the commit message saying so.
With or without that:
Reviewed-by: Gerd Hoffmann 

take care,
  Gerd




[PULL 0/2] Edk2 stable202402 20240226 patches

2024-02-26 Thread Gerd Hoffmann
The following changes since commit 4a4efae44f19528589204581e9e2fab69c5d39aa:

  Merge tag 'pull-hex-20240121' of https://github.com/quic/qemu into staging 
(2024-01-23 13:40:45 +)

are available in the Git repository at:

  https://gitlab.com/kraxel/qemu.git 
tags/edk2-stable202402-20240226-pull-request

for you to fetch changes up to 658178c3d4e95b3f4106e25ec5a209356e339032:

  update edk2 binaries to edk2-stable202402 (2024-02-26 10:23:25 +0100)


firmware: update to edk2-stable202402

Update edk2 to the latest release tagged end of last week.

Cc stable this time because we should move away from the
git snapshot update done in January.



Gerd Hoffmann (2):
  update edk2 submodule to edk2-stable202402
  update edk2 binaries to edk2-stable202402

 pc-bios/edk2-aarch64-code.fd.bz2   | Bin 1587761 -> 1589320 bytes
 pc-bios/edk2-arm-code.fd.bz2   | Bin 1569509 -> 1571418 bytes
 pc-bios/edk2-i386-code.fd.bz2  | Bin 1773520 -> 1775832 bytes
 pc-bios/edk2-i386-secure-code.fd.bz2   | Bin 2127480 -> 2130741 bytes
 pc-bios/edk2-riscv-code.fd.bz2 | Bin 1180314 -> 1345420 bytes
 pc-bios/edk2-riscv-vars.fd.bz2 | Bin 231 -> 235 bytes
 pc-bios/edk2-x86_64-code.fd.bz2| Bin 1891942 -> 1892372 bytes
 pc-bios/edk2-x86_64-microvm.fd.bz2 | Bin 1783951 -> 1785258 bytes
 pc-bios/edk2-x86_64-secure-code.fd.bz2 | Bin 2212258 -> 2214892 bytes
 roms/edk2  |   2 +-
 10 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.43.2




[PULL 1/2] update edk2 submodule to edk2-stable202402

2024-02-26 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 roms/edk2 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roms/edk2 b/roms/edk2
index b8a3eec88cc7..edc6681206c1 16
--- a/roms/edk2
+++ b/roms/edk2
@@ -1 +1 @@
-Subproject commit b8a3eec88cc74bbfe7fb389d026cc7d1d8a989c8
+Subproject commit edc6681206c1a8791981a2f911d2fb8b3d2f5768
-- 
2.43.2




Re: [PATCH v3] pc: q35: Bump max_cpus to 4096 vcpus

2024-02-21 Thread Gerd Hoffmann
  Hi,

> Looking at the edk2 GH, are these the PR that are waiting for upstream 
> review/merge that relate to vcpu scaling?
> 
> https://github.com/tianocore/edk2/pull/5375
> https://github.com/tianocore/edk2/pull/5327

These are draft MRs for running CI.

The current devel branches are:
  https://github.com/kraxel/edk2/tree/devel/many-vcpus
  https://github.com/kraxel/edk2/tree/devel/many-vcpus-mpinitlib

All of them will expire at some point though, so I don't think it is a
good idea to include them in the commit message.  They will point into
nowhere in a year or so.

take care,
  Gerd




Re: [PATCH v3] pc: q35: Bump max_cpus to 4096 vcpus

2024-02-20 Thread Gerd Hoffmann
On Tue, Feb 20, 2024 at 03:50:57PM +, Daniel P. Berrangé wrote:
> On Tue, Feb 20, 2024 at 09:12:04PM +0530, Ani Sinha wrote:
> > Since commit f10a570b093e6 ("KVM: x86: Add CONFIG_KVM_MAX_NR_VCPUS to allow 
> > up to 4096 vCPUs")
> > Linux kernel can support upto a maximum number of 4096 vCPUS when MAXSMP is
> > enabled in the kernel. At present, QEMU has been tested to correctly boot a
> > linux guest with 4096 vcpus both with edk2 and seabios firmwares.
> > So bump up the value max_cpus to 4096 for q35 machines versions 9 and newer.
> > Q35 machines versions 8.2 and older continue to support 1024 maximum vcpus
> > as before for compatibility reasons.
> > 
> > If KVM is not able to support the specified number of vcpus, QEMU would
> > return the following error messages:
> > 
> > $ ./qemu-system-x86_64 -cpu host -accel kvm -machine q35 -smp 1728
> > qemu-system-x86_64: -accel kvm: warning: Number of SMP cpus requested 
> > (1728) exceeds the recommended cpus supported by KVM (12)
> > qemu-system-x86_64: -accel kvm: warning: Number of hotpluggable cpus 
> > requested (1728) exceeds the recommended cpus supported by KVM (12)
> > Number of SMP cpus requested (1728) exceeds the maximum cpus supported by 
> > KVM (1024)
> > 
> > Cc: Daniel P. Berrangé 
> > Cc: Igor Mammedov 
> > Cc: Michael S. Tsirkin 
> > Cc: Julia Suvorova 
> > Cc: kra...@redhat.com
> > Signed-off-by: Ani Sinha 
> > ---
> >  hw/i386/pc_q35.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Changelog:
> > v3: bump up to 4096 vcpus. It has now been tested to work with edk2.
> > See RH Jira: https://issues.redhat.com/browse/RHEL-22202
> 
> That bug indicates a dependancy on a EDK2 patch
> 
>   
> https://github.com/kraxel/edk2/commit/7a03c17f0f4f4a9003d77db2660c8e087604b2f0
> 
> we'll need to rebase the EDK2 ROMs in QEMU to get that included.

Which will btw take a while.  edk2 is in freeze for the 2024-02 release
right now, I expect the changes land upstream shortly thereafter and
will be part of the 2024-05 release.  So end may / early june would be
the time when rebasing to release, or somewhen in march or april when we
rebase to a git snapshot ...

> Meanwhile, plesae at least call out this EDK2 commit as a
> pre-requisite in the commit message, so people know the
> EDK2 ROMs in QEMU won't work (yet).

That surely makes sense.

Oh, and it's more than just that one commit.  I don't think it makes
sense to compile a list of commits given this is a moving target
(upstream review is in progress right now).

take care,
  Gerd




Re: Re: why various devices are loading x86 roms on non-x86 architectures?

2024-02-01 Thread Gerd Hoffmann
  Hi,

> There are also some PPC machines that have BIOS emulator in firmware and
> would need the ROM to init the device but these have problems with QEMU's
> gcc compiled ROMs that contain i386 opcodes and can't run these.

That used to be standard practice in linux.  Before we got kernel mode
setting the Xserver ran the vgabios code to set video modes.  On i386 in
vm86 mode, on all other architectures (including x86_64) in an emulator
(x86emu fork IIRC).

Fun fact:  There are a bunch of hacks in seabios to avoid certain
instructions in vgabios binaries, to workaround emulator limitations.
And, yes, i386 opcodes are problematic, even though at least the Xserver
emulator became better over time.

take care,
  Gerd




  1   2   3   4   5   6   7   8   9   10   >