Re: [v2 1/1] hw/i386/acpi-build: add OSHP method support for SHPC driver load
Hi, > > for SHPC on PXB see, > > commit d10dda2d60 hw/pci-bridge: disable SHPC in PXB > > > > it seems that enabling SHPC on PXB in QEMU is not enough, > > UEFI needs to support that as well > > (CCing Gerd to check whether it is possible at all) Hmm, can't give a quick answer on that. From the commit message it doesn't look easy ... > > > If I want to use ACPI PCI hotplug in the pxb bridge, what else need to be > > > done? > > > > does it have to be hotplug directly into pxb or > > would be it be sufficient to have hotplug support > > on pci-bridge attached to a pxb? > > It's sufficient to hotplug support on pci-bridge attached to a pxb. ... but I guess using this instead would be better anyway? take care, Gerd
Re: [PATCH v3 02/15] uefi-test-tools: Add support for python based build script
On Fri, Jun 21, 2024 at 05:28:53PM GMT, Sunil V L wrote:
> edk2-funcs.sh which is used in this Makefile, was removed in the commit
> c28a2891f3 ("edk2: update build script"). It is replaced with a python
> based script. So, update the Makefile and add the configuration file as
> required to support the python based build script.
>
> Signed-off-by: Sunil V L
Acked-by: Gerd Hoffmann
Re: [PATCH v3 2/4] usb/hub: mark as deprecated
Hi, > > This does seem quite aggressive because there may be cases when users > > explicitly want to use old devices. Maybe there is need for a third > > state (better_alternatives?) so we can steer users away from old command > > lines they may have picked up from the web to the modern alternative? > > We've got 2 flags proposed in patch 1 - "deprecated" and "not_secure" - > which we formally expose to mgmt apps/users. Both of these are actionable > in an unambiguous way. An application can query this info, and can also > tell QEMU to enforce policy on this. Both are good. Well, given that people apparently don't want actually delete stuff I'm not sure the 'deprecated' flag actually makes sense. > The "better alternatives" conceptable, however, is an inherantly fuzzy > concept, because "better" is very much a use-case depedent / eye of the > beholder concept. Well. I think the use cases can be grouped into two cases: (1) Operating system museum. People using qemu qemu to keep their beloved (historical) OS alive on modern hardware. (2) Virtualization. Run your production workload in VMs. For (1) "better alternatives" doesn't make sense because you have to consider what your (old) guest OS supports. For (2) I think it does make sense. I'd be conservative here and would define possible production workloads as "OS which is still supported with security updates". Hardware old enough to be supported by all production workloads (which should roughly being 10-15 years in the market) can go into the "better alternative" bucket. Examples: - xhci is old enough and can replace ohci/uhci/ehci. - intel-hda is old enough and can replace ac97/es1370/sb/... - sata is old enough and can replace ide. - nvme isn't old enough yet I'd say (also no cdrom support so it can't fully replace ide/sata). - q35 is old enough and can replace pc. > This would makes it difficult/impractical for an application to take > action based on such a "better-alternatives' schema marker. It would > imply the application has to understands the better alternatives ahead > of time, as well as understanding the end users' intent and that's not > really viable. Well, with the conservative classification it should be possible to simply apply it universally. Throw warnings or errors in case a device with a "better-alternative" tag is used. Or build qemu without these devices. Or move these devices to a sub-rpm (if modularized), so they are not present by default but can be installed for the (1) use cases. take care, Gerd
Re: [PATCH v3 2/4] usb/hub: mark as deprecated
On Thu, Jun 13, 2024 at 03:49:23PM GMT, Alex Bennée wrote: > Daniel P. Berrangé writes: > > > I don't want to loose that clear & easily understood meaning, by overloading > > "deprecated" for scenarios like "it is sometimes better to use a different > > device instead of this one, depending on factors X, Y & Z". > > I agree we shouldn't overload the meaning if deprecated. > > So to this case in point. Is there anything you can do with usb/hub that > you can't do with the newer xhci based one? Well, it's a hub. You can (virtually) plug it into a usb root port of a ohci/uhci/ehci/xhci host adapter and get 8 more usb ports. Those new ports are usb 1.1 only though, so not very useful in a modern world, except maybe for HID devices which don't need much bandwidth. > Is it backward compatible > enough that an old kernel would work? Or are we talking really old > kernels at this point? Pretty much everything should be able to drive the usb hub. take care, Gerd
[PATCH v3 1/4] qom: allow to mark objects as deprecated or not secure.
Add flags to ObjectClass for objects which are deprecated or not secure.
Add 'deprecated' and 'not-secure' bools to ObjectTypeInfo, report in
'qom-list-types'. Print the flags when listing devices via '-device
help'.
Signed-off-by: Gerd Hoffmann
---
include/qom/object.h | 3 +++
qom/qom-qmp-cmds.c| 8
system/qdev-monitor.c | 8
qapi/qom.json | 8 +++-
4 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 13d3a655ddf9..419bd9a4b219 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -136,6 +136,9 @@ struct ObjectClass
ObjectUnparent *unparent;
GHashTable *properties;
+
+bool deprecated;
+bool not_secure;
};
/**
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index e91a2353472a..325ff0ba2a25 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -101,6 +101,14 @@ static void qom_list_types_tramp(ObjectClass *klass, void
*data)
if (parent) {
info->parent = g_strdup(object_class_get_name(parent));
}
+if (klass->deprecated) {
+info->has_deprecated = true;
+info->deprecated = true;
+}
+if (klass->not_secure) {
+info->has_not_secure = true;
+info->not_secure = true;
+}
QAPI_LIST_PREPEND(*pret, info);
}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d667f..effdc95d21d3 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,12 @@ static void qdev_print_devinfo(DeviceClass *dc)
if (!dc->user_creatable) {
qemu_printf(", no-user");
}
+if (klass->deprecated) {
+qemu_printf(", deprecated");
+}
+if (klass->not_secure) {
+qemu_printf(", not-secure");
+}
qemu_printf("\n");
}
diff --git a/qapi/qom.json b/qapi/qom.json
index 8bd299265e39..3f20d4c6413b 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -163,10 +163,16 @@
#
# @parent: Name of parent type, if any (since 2.10)
#
+# @deprecated: the type is deprecated (since 9.1)
+#
+# @not-secure: the type (typically a device) is not considered
+# a security boundary (since 9.1)
+#
# Since: 1.1
##
{ 'struct': 'ObjectTypeInfo',
- 'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str' } }
+ 'data': { 'name': 'str', '*abstract': 'bool', '*parent': 'str',
+'*deprecated': 'bool', '*not-secure': 'bool' } }
##
# @qom-list-types:
--
2.45.2
[PATCH v3 4/4] qdev: add device policy [RfC]
Add policies for devices which are deprecated or not secure.
There are three options: allow, warn and deny.
It's implemented for devices only. Devices will probably be the main
user of this. Also object_new() can't fail as of today so it's a bit
hard to implement policy checking at object level, especially the 'deny'
part of it.
TODO: add a command line option to actually set these policies.
Comments are welcome.
Signed-off-by: Gerd Hoffmann
---
hw/core/qdev.c | 60 +-
1 file changed, 59 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f3a996f57dee..0c4e5cec743c 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -43,6 +43,15 @@
static bool qdev_hot_added = false;
bool qdev_hot_removed = false;
+enum qdev_policy {
+QDEV_ALLOW = 0,
+QDEV_WARN = 1,
+QDEV_DENY = 2,
+};
+
+static enum qdev_policy qdev_deprecated_policy;
+static enum qdev_policy qdev_not_secure_policy;
+
const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
{
DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -144,6 +153,43 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus,
Error **errp)
return true;
}
+static bool qdev_class_check(const char *name, ObjectClass *oc)
+{
+bool allow = true;
+
+if (oc->deprecated) {
+switch (qdev_deprecated_policy) {
+case QDEV_WARN:
+warn_report("device \"%s\" is deprecated", name);
+break;
+case QDEV_DENY:
+error_report("device \"%s\" is deprecated", name);
+allow = false;
+break;
+default:
+/* nothing */
+break;
+}
+}
+
+if (oc->not_secure) {
+switch (qdev_not_secure_policy) {
+case QDEV_WARN:
+warn_report("device \"%s\" is not secure", name);
+break;
+case QDEV_DENY:
+error_report("device \"%s\" is not secure", name);
+allow = false;
+break;
+default:
+/* nothing */
+break;
+}
+}
+
+return allow;
+}
+
DeviceState *qdev_new(const char *name)
{
ObjectClass *oc = object_class_by_name(name);
@@ -162,14 +208,26 @@ DeviceState *qdev_new(const char *name)
error_report("unknown type '%s'", name);
abort();
}
+
+if (!qdev_class_check(name, oc)) {
+exit(1);
+}
+
return DEVICE(object_new(name));
}
DeviceState *qdev_try_new(const char *name)
{
-if (!module_object_class_by_name(name)) {
+ObjectClass *oc = module_object_class_by_name(name);
+
+if (!oc) {
return NULL;
}
+
+if (!qdev_class_check(name, oc)) {
+return NULL;
+}
+
return DEVICE(object_new(name));
}
--
2.45.2
[PATCH v3 0/4] allow to deprecate objects and devices
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. v3: - switch to two properties: 'deprecated' and 'not secure' flags. - add rfc patch implementing policies for devices with flags. v2: - pick up reviews. - drop ohci patch. - add cirrus vga patch. Gerd Hoffmann (4): qom: allow to mark objects as deprecated or not secure. usb/hub: mark as deprecated vga/cirrus: mark as not secure qdev: add device policy [RfC] include/qom/object.h| 3 ++ hw/core/qdev.c | 60 - hw/display/cirrus_vga.c | 1 + hw/display/cirrus_vga_isa.c | 1 + hw/usb/dev-hub.c| 1 + qom/qom-qmp-cmds.c | 8 + system/qdev-monitor.c | 8 + qapi/qom.json | 8 - 8 files changed, 88 insertions(+), 2 deletions(-) -- 2.45.2
[PATCH v3 3/4] vga/cirrus: mark as not secure
Signed-off-by: Gerd Hoffmann
---
hw/display/cirrus_vga.c | 1 +
hw/display/cirrus_vga_isa.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 150883a97166..1f4c55b21415 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->not_secure = true;
}
static const TypeInfo cirrus_vga_info = {
diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
index 84be51670ed8..535a631b4b09 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->not_secure = true;
}
static const TypeInfo isa_cirrus_vga_info = {
--
2.45.2
[PATCH v3 2/4] usb/hub: mark as deprecated
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 + 1 file changed, 1 insertion(+) diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c index 06e9537d0356..bc8d0ba4cfcf 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->deprecated = true; device_class_set_props(dc, usb_hub_properties); } -- 2.45.2
[PATCH v3 1/3] stdvga: fix screen blanking
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..474b6b14c327 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 */
+surface = qemu_create_displaysurface(s->last_scr_width,
s->last_scr_height);
+dpy_gfx_replace_surface(s->con, surface);
+}
+
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
[PATCH v3 2/3] ui+display: rename is_placeholder() -> surface_is_placeholder()
No functional change.
Signed-off-by: Gerd Hoffmann
---
include/ui/surface.h | 2 +-
ui/console.c | 2 +-
ui/sdl2-2d.c | 2 +-
ui/sdl2-gl.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/ui/surface.h b/include/ui/surface.h
index 4244e0ca4a32..273bb4769a02 100644
--- a/include/ui/surface.h
+++ b/include/ui/surface.h
@@ -50,7 +50,7 @@ static inline int is_buffer_shared(DisplaySurface *surface)
return !(surface->flags & QEMU_ALLOCATED_FLAG);
}
-static inline int is_placeholder(DisplaySurface *surface)
+static inline int surface_is_placeholder(DisplaySurface *surface)
{
return surface->flags & QEMU_PLACEHOLDER_FLAG;
}
diff --git a/ui/console.c b/ui/console.c
index 1b2cd0c7365d..c2173fc0b1e5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1510,7 +1510,7 @@ void qemu_console_resize(QemuConsole *s, int width, int
height)
assert(QEMU_IS_GRAPHIC_CONSOLE(s));
if ((s->scanout.kind != SCANOUT_SURFACE ||
- (surface && !is_buffer_shared(surface) && !is_placeholder(surface)))
&&
+ (surface && !is_buffer_shared(surface) &&
!surface_is_placeholder(surface))) &&
qemu_console_get_width(s, -1) == width &&
qemu_console_get_height(s, -1) == height) {
return;
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 06468cd493ea..73052383c2e0 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -72,7 +72,7 @@ void sdl2_2d_switch(DisplayChangeListener *dcl,
scon->texture = NULL;
}
-if (is_placeholder(new_surface) && qemu_console_get_index(dcl->con)) {
+if (surface_is_placeholder(new_surface) &&
qemu_console_get_index(dcl->con)) {
sdl2_window_destroy(scon);
return;
}
diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index 28d796607c08..91b7ee2419b7 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -89,7 +89,7 @@ void sdl2_gl_switch(DisplayChangeListener *dcl,
scon->surface = new_surface;
-if (is_placeholder(new_surface) && qemu_console_get_index(dcl->con)) {
+if (surface_is_placeholder(new_surface) &&
qemu_console_get_index(dcl->con)) {
qemu_gl_fini_shader(scon->gls);
scon->gls = NULL;
sdl2_window_destroy(scon);
--
2.45.1
[PATCH v3 0/3] stdvga: fix screen blanking
Gerd Hoffmann (3): stdvga: fix screen blanking ui+display: rename is_placeholder() -> surface_is_placeholder() ui+display: rename is_buffer_shared() -> surface_is_allocated() include/ui/surface.h| 6 +++--- hw/display/qxl-render.c | 2 +- hw/display/vga.c| 24 +++- hw/display/xenfb.c | 5 +++-- ui/console.c| 3 ++- ui/sdl2-2d.c| 2 +- ui/sdl2-gl.c| 2 +- 7 files changed, 26 insertions(+), 18 deletions(-) -- 2.45.1
[PATCH v3 3/3] ui+display: rename is_buffer_shared() -> surface_is_allocated()
Boolean return value is reversed, to align with QEMU_ALLOCATED_FLAG, so
all callers must be adapted. Also rename share_surface variable in
vga_draw_graphic() to reduce confusion.
No functional change.
Suggested-by: Marc-André Lureau
Signed-off-by: Gerd Hoffmann
---
include/ui/surface.h| 4 ++--
hw/display/qxl-render.c | 2 +-
hw/display/vga.c| 20 ++--
hw/display/xenfb.c | 5 +++--
ui/console.c| 3 ++-
5 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/include/ui/surface.h b/include/ui/surface.h
index 273bb4769a02..345b19169d2e 100644
--- a/include/ui/surface.h
+++ b/include/ui/surface.h
@@ -45,9 +45,9 @@ void qemu_displaysurface_win32_set_handle(DisplaySurface
*surface,
DisplaySurface *qemu_create_displaysurface(int width, int height);
void qemu_free_displaysurface(DisplaySurface *surface);
-static inline int is_buffer_shared(DisplaySurface *surface)
+static inline int surface_is_allocated(DisplaySurface *surface)
{
-return !(surface->flags & QEMU_ALLOCATED_FLAG);
+return surface->flags & QEMU_ALLOCATED_FLAG;
}
static inline int surface_is_placeholder(DisplaySurface *surface)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index ec99ec887a6e..8daae72c8d04 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -31,7 +31,7 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
uint8_t *src;
int len, i;
-if (is_buffer_shared(surface)) {
+if (!surface_is_allocated(surface)) {
return;
}
trace_qxl_render_blit(qxl->guest_primary.qxl_stride,
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 474b6b14c327..0ed933596584 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1487,7 +1487,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
uint8_t *d;
uint32_t v, addr1, addr;
vga_draw_line_func *vga_draw_line = NULL;
-bool share_surface, force_shadow = false;
+bool allocate_surface, force_shadow = false;
pixman_format_code_t format;
#if HOST_BIG_ENDIAN
bool byteswap = !s->big_endian_fb;
@@ -1609,10 +1609,10 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
*/
format = qemu_default_pixman_format(depth, !byteswap);
if (format) {
-share_surface = dpy_gfx_check_format(s->con, format)
-&& !s->force_shadow && !force_shadow;
+allocate_surface = !dpy_gfx_check_format(s->con, format)
+|| s->force_shadow || force_shadow;
} else {
-share_surface = false;
+allocate_surface = true;
}
if (s->params.line_offset != s->last_line_offset ||
@@ -1620,7 +1620,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
height != s->last_height ||
s->last_depth != depth ||
s->last_byteswap != byteswap ||
-share_surface != is_buffer_shared(surface)) {
+allocate_surface != surface_is_allocated(surface)) {
/* display parameters changed -> need new display surface */
s->last_scr_width = disp_width;
s->last_scr_height = height;
@@ -1635,14 +1635,14 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
full_update = 1;
}
if (surface_data(surface) != s->vram_ptr + (s->params.start_addr * 4)
-&& is_buffer_shared(surface)) {
+&& !surface_is_allocated(surface)) {
/* base address changed (page flip) -> shared display surfaces
* must be updated with the new base address */
full_update = 1;
}
if (full_update) {
-if (share_surface) {
+if (!allocate_surface) {
surface = qemu_create_displaysurface_from(disp_width,
height, format, s->params.line_offset,
s->vram_ptr + (s->params.start_addr * 4));
@@ -1655,7 +1655,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
vga_draw_line = vga_draw_line_table[v];
-if (!is_buffer_shared(surface) && s->cursor_invalidate) {
+if (surface_is_allocated(surface) && s->cursor_invalidate) {
s->cursor_invalidate(s);
}
@@ -1707,7 +1707,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
if (update) {
if (y_start < 0)
y_start = y;
-if (!(is_buffer_shared(surface))) {
+if (surface_is_allocated(surface)) {
uint8_t *p;
p = vga_draw_line(s, d, addr, width, hpel);
if (p) {
@@ -1762,7 +1762,7 @@ 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)) {
+if (!surface_is_allocated(surface)) {
/
Re: [PATCH v2 1/3] stdvga: fix screen blanking
On Tue, Jun 04, 2024 at 10:27:18AM GMT, Marc-André Lureau wrote:
> Hi
>
> > +if (is_buffer_shared(surface)) {
>
> Perhaps the suggestion to rename the function (in the following patch)
> should instead be surface_is_allocated() ? that would match the actual
> flag check. But callers would have to ! the result. Wdyt?
surface_is_shadow() ? Comes closer to the typical naming in computer
graphics.
take care,
Gerd
Re: [PATCH v2 4/4] vga/cirrus: deprecate, don't build by default
Hi, > > Upstream QEMU's scope is to emulate pretty much arbitrary hardware that > > may have existed at any point in time. Emulating Cirrus is very much > > in scope upstream, and even if there are other better VGA devices, that > > doesn't make emulation of Cirrus redundant. > > > > Downstream is a different matter - if a downstream vendor wants to be > > opinionated and limit the scope of devices they ship to customers, it > > is totally valid to cull Cirrus. > > Few years ago I suggested qemu_security_policy_taint() "which allows > unsafe (read "not very maintained") code to 'taint' QEMU security > policy." (Gerd FYI): > https://lore.kernel.org/qemu-devel/20210908232024.2399215-1-phi...@redhat.com/ > > Upstream we could add a boolean in DeviceClass about device security > status / maintenance (or enum or bitfield); then downstreams could > use it to be sure unsafe devices aren't linked in. Yes, I think it makes sense to maintain that information upstream. It is useful information to have. Even if upstream isn't going to enforce something qemu could print useful hints. So, the question is whenever we want go for a simple bool, or go for a bitfield giving more detailed information. Bits I think could be useful: (1) OBJECT_STATUS_DEPRECATED Stuff we actually want remove. Very few cases, maybe usb-hub. (2) OBJECT_STATUS_UNSAFE "not very maintained". Probably need a better name for this. (3) OBJECT_STATUS_OUTDATED Old device where newer / better alternatives exist. Looking at the USB host adapters I'd attach 2+3 to ohci and 3 to uhci and ehci. In general there is a lot of overlap for (2) + (3) though. We might also have recommendation bits such as: (4) OBJECT_STATUS_VIRTUALIZATON Devices recommended for virtualization use cases (all virtio, xhci, ...). > > IOW, I think device deprecation *framework* is relevant to include > > upstream, but most actual usage of it will be downstream. When doing it at ObjectClass not DeviceClass level we get introspection support for (almost) free, so I'd do it for ObjectClass even if the vast majority of users will be devices. > > Upstream might use *object* deprecation, if we replace an backend > > implementation with a different one. Sounds like having OBJECT_STATUS_DEPRECATED makes sense even if we don't have any device actually using that. take care, Gerd
[PATCH v2 3/3] ui+display: rename is_placeholder -> surface_is_placeholder
No functional change.
Signed-off-by: Gerd Hoffmann
---
include/ui/surface.h | 2 +-
ui/console.c | 2 +-
ui/sdl2-2d.c | 2 +-
ui/sdl2-gl.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/ui/surface.h b/include/ui/surface.h
index 96f9b1611c1c..60416a451901 100644
--- a/include/ui/surface.h
+++ b/include/ui/surface.h
@@ -50,7 +50,7 @@ static inline int surface_is_borrowed(DisplaySurface *surface)
return !(surface->flags & QEMU_ALLOCATED_FLAG);
}
-static inline int is_placeholder(DisplaySurface *surface)
+static inline int surface_is_placeholder(DisplaySurface *surface)
{
return surface->flags & QEMU_PLACEHOLDER_FLAG;
}
diff --git a/ui/console.c b/ui/console.c
index d7967ddb0d1a..3bd2adcc33c3 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1510,7 +1510,7 @@ void qemu_console_resize(QemuConsole *s, int width, int
height)
assert(QEMU_IS_GRAPHIC_CONSOLE(s));
if ((s->scanout.kind != SCANOUT_SURFACE ||
- (surface && !surface_is_borrowed(surface) &&
!is_placeholder(surface))) &&
+ (surface && !surface_is_borrowed(surface) &&
!surface_is_placeholder(surface))) &&
qemu_console_get_width(s, -1) == width &&
qemu_console_get_height(s, -1) == height) {
return;
diff --git a/ui/sdl2-2d.c b/ui/sdl2-2d.c
index 06468cd493ea..73052383c2e0 100644
--- a/ui/sdl2-2d.c
+++ b/ui/sdl2-2d.c
@@ -72,7 +72,7 @@ void sdl2_2d_switch(DisplayChangeListener *dcl,
scon->texture = NULL;
}
-if (is_placeholder(new_surface) && qemu_console_get_index(dcl->con)) {
+if (surface_is_placeholder(new_surface) &&
qemu_console_get_index(dcl->con)) {
sdl2_window_destroy(scon);
return;
}
diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
index 28d796607c08..91b7ee2419b7 100644
--- a/ui/sdl2-gl.c
+++ b/ui/sdl2-gl.c
@@ -89,7 +89,7 @@ void sdl2_gl_switch(DisplayChangeListener *dcl,
scon->surface = new_surface;
-if (is_placeholder(new_surface) && qemu_console_get_index(dcl->con)) {
+if (surface_is_placeholder(new_surface) &&
qemu_console_get_index(dcl->con)) {
qemu_gl_fini_shader(scon->gls);
scon->gls = NULL;
sdl2_window_destroy(scon);
--
2.45.1
[PATCH v2 0/3] stdvga: fix screen blanking
Gerd Hoffmann (3): stdvga: fix screen blanking ui+display: rename is_buffer_shared() -> surface_is_borrowed() ui+display: rename is_placeholder -> surface_is_placeholder include/ui/surface.h| 4 ++-- hw/display/qxl-render.c | 2 +- hw/display/vga.c| 14 ++ hw/display/xenfb.c | 4 ++-- ui/console.c| 2 +- ui/sdl2-2d.c| 2 +- ui/sdl2-gl.c| 2 +- 7 files changed, 18 insertions(+), 12 deletions(-) -- 2.45.1
[PATCH v2 1/3] stdvga: fix screen blanking
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..474b6b14c327 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 */
+surface = qemu_create_displaysurface(s->last_scr_width,
s->last_scr_height);
+dpy_gfx_replace_surface(s->con, surface);
+}
+
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
[PATCH v2 2/3] ui+display: rename is_buffer_shared() -> surface_is_borrowed()
No functional change.
Suggested-by: Marc-André Lureau
Signed-off-by: Gerd Hoffmann
---
include/ui/surface.h| 2 +-
hw/display/qxl-render.c | 2 +-
hw/display/vga.c| 10 +-
hw/display/xenfb.c | 4 ++--
ui/console.c| 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/ui/surface.h b/include/ui/surface.h
index 4244e0ca4a32..96f9b1611c1c 100644
--- a/include/ui/surface.h
+++ b/include/ui/surface.h
@@ -45,7 +45,7 @@ void qemu_displaysurface_win32_set_handle(DisplaySurface
*surface,
DisplaySurface *qemu_create_displaysurface(int width, int height);
void qemu_free_displaysurface(DisplaySurface *surface);
-static inline int is_buffer_shared(DisplaySurface *surface)
+static inline int surface_is_borrowed(DisplaySurface *surface)
{
return !(surface->flags & QEMU_ALLOCATED_FLAG);
}
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index ec99ec887a6e..bfdf2c59bdbe 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -31,7 +31,7 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
uint8_t *src;
int len, i;
-if (is_buffer_shared(surface)) {
+if (surface_is_borrowed(surface)) {
return;
}
trace_qxl_render_blit(qxl->guest_primary.qxl_stride,
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 474b6b14c327..bd800a683e45 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1620,7 +1620,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
height != s->last_height ||
s->last_depth != depth ||
s->last_byteswap != byteswap ||
-share_surface != is_buffer_shared(surface)) {
+share_surface != surface_is_borrowed(surface)) {
/* display parameters changed -> need new display surface */
s->last_scr_width = disp_width;
s->last_scr_height = height;
@@ -1635,7 +1635,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
full_update = 1;
}
if (surface_data(surface) != s->vram_ptr + (s->params.start_addr * 4)
-&& is_buffer_shared(surface)) {
+&& surface_is_borrowed(surface)) {
/* base address changed (page flip) -> shared display surfaces
* must be updated with the new base address */
full_update = 1;
@@ -1655,7 +1655,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
vga_draw_line = vga_draw_line_table[v];
-if (!is_buffer_shared(surface) && s->cursor_invalidate) {
+if (!surface_is_borrowed(surface) && s->cursor_invalidate) {
s->cursor_invalidate(s);
}
@@ -1707,7 +1707,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
if (update) {
if (y_start < 0)
y_start = y;
-if (!(is_buffer_shared(surface))) {
+if (!(surface_is_borrowed(surface))) {
uint8_t *p;
p = vga_draw_line(s, d, addr, width, hpel);
if (p) {
@@ -1762,7 +1762,7 @@ 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)) {
+if (surface_is_borrowed(surface)) {
/* unshare buffer, otherwise the blanking corrupts vga vram */
surface = qemu_create_displaysurface(s->last_scr_width,
s->last_scr_height);
dpy_gfx_replace_surface(s->con, surface);
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 27536bfce0cb..a9bc5a08cdc2 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -637,7 +637,7 @@ static void xenfb_guest_copy(struct XenFB *xenfb, int x,
int y, int w, int h)
int linesize = surface_stride(surface);
uint8_t *data = surface_data(surface);
-if (!is_buffer_shared(surface)) {
+if (!surface_is_borrowed(surface)) {
switch (xenfb->depth) {
case 8:
if (bpp == 16) {
@@ -755,7 +755,7 @@ static void xenfb_update(void *opaque)
xen_pv_printf(>c.xendev, 1,
"update: resizing: %dx%d @ %d bpp%s\n",
xenfb->width, xenfb->height, xenfb->depth,
- is_buffer_shared(surface) ? " (shared)" : "");
+ surface_is_borrowed(surface) ? " (borrowed)" : "");
xenfb->up_fullscreen = 1;
}
diff --git a/ui/console.c b/ui/console.c
index 1b2cd0c7365d..d7967ddb0d1a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1510,7 +1510,7 @@ void qemu_console_resize(QemuConsole *s, int width, int
height)
assert(QEMU_IS_GRAPHIC_CONSOLE(s));
if ((s->scanout.kind != SCANOUT_SURFACE ||
- (surface && !is_buffer_shared(surface) && !is_placeholder(surface)))
&&
+ (surface && !surface_is_borrowed(surface) &&
!is_placeholder(surface))) &&
qemu_console_get_width(s, -1) == width &&
qemu_console_get_height(s, -1) == height) {
return;
--
2.45.1
Re: [PATCH] stdvga: fix screen blanking
On Mon, Jun 03, 2024 at 02:24:52PM GMT, Marc-André Lureau wrote:
> Hi
>
> On Thu, May 30, 2024 at 3:05 PM Gerd Hoffmann wrote:
>
> > 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)) {
> >
>
> It might be a good time to rename this function. surface_is_borrowed() ?
"shared" means memory shared between guest and host (typically vga vram).
I doubt using the term "borrowed" instead clarifies things much,
especially as this isn't an rust-style "borrow" (which I guess you are
referring to). Nothing prevents the host from writing to the surface as
the bug clearly shows. Also qemu is a C project, so I wouldn't expect
developers being familiar with rust semantics and terminology.
> > +/* unshare buffer, otherwise the blanking corrupts vga vram */
> > +qemu_console_resize(s->con, s->last_scr_width,
> > s->last_scr_height);
>
> If we want to guarantee that a new surface is created, we should leave a
> comment on qemu_console_resize(),
I left the comment there exactly because it isn't obvious that the
qemu_console_resize() will create a new (not shared) surface. So not
sure what exactly you are suggesting here?
> or perhaps make it take a new/alloc argument?
Right now qemu_console_resize() does a bunch of checks to figure
whenever it can take a shortcut (because width + height didn't change)
or not.
We could certainly pass a boolean in instead and have the caller decide
that way. Didn't check whenever that makes sense, and IMHO that is well
beyond the scope of a 3-lines bugfix.
kraxel@sirius ~/projects/qemu# git grep qemu_console_resize | wc -l
35
take care,
Gerd
Re: [PATCH 4/5] x86/loader: expose unpatched kernel
On Sun, Jun 02, 2024 at 09:26:09AM GMT, Michael S. Tsirkin wrote: > On Thu, Apr 11, 2024 at 11:48:28AM +0200, Gerd Hoffmann wrote: > > 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 > > So given we have this, do we still need patch 2? With this merged to qemu plus related edk2 patches merged too OVMF will stop using the patched linux kernel setup header fw_cfg file. So, patch #2 will not be essential for direct kernel boot to work correctly with UEFI. Nevertheless I'd consider patch #2 a clear bugfix. Trying to patch linux kernel setup header fields in binaries which are /not/ a linux kernel doesn't make any sense. take care, Gerd
Re: [PATCH v2 4/4] vga/cirrus: deprecate, don't build by default
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
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.
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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.
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
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
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
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
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
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.
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
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
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
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
> 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
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)
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()
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
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
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
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
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
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
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()
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)
> > > 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)
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> > +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
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
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
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
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
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
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
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
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
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
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
> +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
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
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
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
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
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
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
