Re: [PATCH] docs/cxl: fix some typos
Hello Jonathan, Thank you for your response. On Sat, Jun 22, 2024 at 1:10 AM Jonathan Cameron wrote: > > On Wed, 19 Jun 2024 13:54:59 +0900 > Hyeongtak Ji wrote: > > Hi, some description would be good of how you caught these > (I'm guessing a close read). Just to confirm, are you suggesting that the patch should include a commit message? I apologize for submitting the patch without any sufficient explanation. However, I am not entirely sure if "how I found these typos" needs to be included in the commit message. For your information, I discovered these typos because the ASCII art did not align with the explanations (yes, a close read). > > Whilst checking this I did notice there are some errors in > the example bus numbering but that's a separate issue. > > Jonathan > > > > Signed-off-by: Hyeongtak Ji > > --- > > docs/system/devices/cxl.rst | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/docs/system/devices/cxl.rst b/docs/system/devices/cxl.rst > > index 10a0e9bc9ff4..e2497e6a098b 100644 > > --- a/docs/system/devices/cxl.rst > > +++ b/docs/system/devices/cxl.rst > > @@ -218,17 +218,17 @@ Notes: > > A complex configuration here, might be to use the following HDM > > decoders in HB0. HDM0 routes CFMW0 requests to RP0 and hence > > part of CXL Type3 0. HDM1 routes CFMW0 requests from a > > -different region of the CFMW0 PA range to RP2 and hence part > > +different region of the CFMW0 PA range to RP1 and hence part > > Good catch. > > > of CXL Type 3 1. HDM2 routes yet another PA range from within > > CFMW0 to be interleaved across RP0 and RP1, providing 2 way > > interleave of part of the memory provided by CXL Type3 0 and > > CXL Type 3 1. HDM3 routes those interleaved accesses from > > CFMW1 that target HB0 to RP 0 and another part of the memory of > > CXL Type 3 0 (as part of a 2 way interleave at the system level > > -across for example CXL Type3 0 and CXL Type3 2. > > +across for example CXL Type3 0 and CXL Type3 1). > This one is wrong. CFMW1 interleaves across both host bridges so we need > a device below HB0 and one below HB1, so CXL type3 2 is a possible choice > (could be CXL type3 3 as well, but that doesn't matter.) Oh, I misunderstood the original explanation. I will correct it just by adding the missing parenthesis instead. > > > HDM4 is used to enable system wide 4 way interleave across all > > the present CXL type3 devices, by interleaving those (interleaved) > > -requests that HB0 receives from from CFMW1 across RP 0 and > > +requests that HB0 receives from CFMW1 across RP 0 and > Good. > > > RP 1 and hence to yet more regions of the memory of the > > attached Type3 devices. Note this is a representative subset > > of the full range of possible HDM decoder configurations in this > I will send V2 with a decent explanation and the corrected typo fix. Kind regards, Hyeongtak On Sat, Jun 22, 2024 at 1:10 AM Jonathan Cameron wrote: > > On Wed, 19 Jun 2024 13:54:59 +0900 > Hyeongtak Ji wrote: > > Hi, some description would be good of how you caught these > (I'm guessing a close read). > > Whilst checking this I did notice there are some errors in > the example bus numbering but that's a separate issue. > > Jonathan > > > > Signed-off-by: Hyeongtak Ji > > --- > > docs/system/devices/cxl.rst | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/docs/system/devices/cxl.rst b/docs/system/devices/cxl.rst > > index 10a0e9bc9ff4..e2497e6a098b 100644 > > --- a/docs/system/devices/cxl.rst > > +++ b/docs/system/devices/cxl.rst > > @@ -218,17 +218,17 @@ Notes: > > A complex configuration here, might be to use the following HDM > > decoders in HB0. HDM0 routes CFMW0 requests to RP0 and hence > > part of CXL Type3 0. HDM1 routes CFMW0 requests from a > > -different region of the CFMW0 PA range to RP2 and hence part > > +different region of the CFMW0 PA range to RP1 and hence part > > Good catch. > > > of CXL Type 3 1. HDM2 routes yet another PA range from within > > CFMW0 to be interleaved across RP0 and RP1, providing 2 way > > interleave of part of the memory provided by CXL Type3 0 and > > CXL Type 3 1. HDM3 routes those interleaved accesses from > > CFMW1 that target HB0 to RP 0 and another part of the memory of > > CXL Type 3 0 (as part of a 2 way interleave at the system level > > -across for example CXL Type3 0 and CXL Type3 2. > > +across for example CXL Type3 0 and CXL Type3 1). > This one is wrong. CFMW1 interleaves across both host bridges so we need > a device below HB0 and one below HB1, so CXL type3 2 is a possible choice > (could be CXL type3 3 as well, but that doesn't matter.) > > > HDM4 is used to enable system wide 4 way interleave across all > > the present CXL type3 devices, by interleaving those (interleaved) > > -requests that HB
Re: standardizing i2c device ids
Patrick Leis writes: > Corey and Peter, > > My team builds lots of configurations for Qemu boards, and one pain point > has been that the qom path for a device depends on the device insertion > order, child[0], child[1] and the like. Yes. Discussed in my "Dynamic & heterogeneous machines, initial configuration: problems" memo, under "Problem 4: The /machine/unattached/ orphanage". Copy of the "Problem 4" part appended for your your convenience. Full memo archived at https://lore.kernel.org/qemu-devel/87o7d1i7ky@pond.sub.org/ > I noticed that the qdev paths for > devices also exist by their device id property. By default, this ends up > being the device type name. Which kind of devices? There are onboard devices and user-created devices. A user-created device's QOM path is "/machine/peripheral/ID" when it was created with a qdev ID, and "/machine/peripheral-anon/device[N]" (where N counts up from zero) when it was created without a qdev ID. N depends on creation order, which is under the user's control. Users can and should avoid relying on their order by supplying an ID. An onboard device's QOM path is chosen by board code. For instance, q35 puts the mch device at "/machine/q35/mch". However, if the board code neglects to put the device anywhere, the system puts it at "/machine/unattached/device[N]" (where N counts up from zero). N depends on creation order. N can change at the drop of a hat. Whether "device[N]" is a stable interface is unclear. It would clearly be a bad one. If (part of) your problem is "/machine/peripheral-anon/device[N]", supply IDs to bypass it. If (part of) your problem is "/machine/unattached/device[N]", all I can offer is the proper solution: fix the board code to put the device in its proper place instead of abandoning it to the "/machine/unattached/" orphanage. > I was wondering if it made sense to override > this with the device type plus the smbus address? I did something similar > with the i2c mux device, to resolve part of this issue. I doubt it. Questions? = Problem 4: The /machine/unattached/ orphanage = Is it okay for a QOM object to have no parent? An object without a parent is not part of the composition tree; it has no canonical path, and object_get_canonical_path() returns null. Such objects can behave in wonky ways. For instance, object_property_set_link() treats a target object without a parent as null. If a linked object somehow loses its parent, object_property_get_link() will return null even though the underlying C pointer still points to the poor orphan. This strongly suggests QOM was designed with the assumption that objects always have a parent, except during initialization (before they are connected to anything) and finalization (when no longer connected to anything). object_property_try_add_child()'s contract seems to confirm this: * Child properties form the composition tree. All objects need to be a child * of another object. Objects can only be a child of one object. Some functions to create objects take the new object's parent as a parameter. Example: object_new_with_props(), object_new_with_propv(), clock_new(), ... Others set a fixed parent. For instance, we always add character backends to "/chardevs/", objects created with object-add in "/objects/", devices created with device_add in "/machine/peripheral/" (with ID) or "/machine/peripheral-anon/" (without ID), ... There are also functions that don't set a parent: object_new(), object_new_with_class(), qdev_new(), qdev_try_new(), ... Setting a parent is the callers job then. Invites misuse. I'm aware of one instance: @current_migration remains without a parent forever. Not all callers care to set a parent themselves. Instead, they rely on the "/machine/unattached/" orphanage: * qdev_connect_gpio_out_named() needs the input pin to have a parent. If it lacks one, it gets added to "/machine/unattached/" with a made-up name. * device_set_realized() ensures realized devices have a parent by adding devices lacking one to "/machine/unattached/" with a made-up name. * portio_list_add() adds a memory region. If the caller doesn't specify the parent, "/machine/unattached/" is assumed. * memory_region_init() adds a memory region, and may set the parent. If the caller requests setting a parent without specifying one, "/machine/unattached/" is assumed. * qemu_create_machine() adds the main system bus to "/machine/unattached/". Except for the last one, the child names depend on execution order. For instance, device_set_realized() uses "device[N]", where N counts up from zero. These brittle, made-up names are visible in QMP QOM introspection. Whether that's a stable interface is unclear. Better not. We don't rely on these names in C. We follow pointers instead. When we replace C code by configuration, we switch from pointers to names. Brittle names become a problem.
Re: [PATCH 04/13] qapi/parser: preserve indentation in QAPIDoc sections
John Snow writes: > On Fri, Jun 21, 2024 at 2:38 AM Markus Armbruster wrote: [...] >> I'd like you to express more clearly that you're talking about an >> alternative you rejected. Perhaps like this: >> >> block-level constructs such as code blocks, lists, and other such >> markup. >> >> The alternative would be to somehow undo .get_doc_indented()'s >> indentation changes in the new generator. Much messier. >> >> Feel free to add more detail to the last paragraph. >> > > Eh, I just deleted it. I recall running into troubles but I can't > articulate the precise conditions because as you point out, it's a doomed > strategy for other reasons - you can't reconstruct the proper indentation. > > This patch is still the correct way to go, so I don't have to explain my > failures at length in the commit message ... I just like giving people > clues for *why* I decided to implement things a certain way, because I > often find that more instructive than the "how". "Why" tends to be much more useful in a commit message than "how". I should be able to figure out "how" by reading the patch, whereas for "why", I may have to read the author's mind. > In this case, the "why" is > probably more properly summarized as "it's a total shitshow in that > direction, trust me" The right amount of detail is often not obvious. Use your judgement.
Re: [PATCH 09/13] qapi: convert "Note" sections to plain rST
John Snow writes: > On Fri, Jun 21, 2024 at 8:23 AM Markus Armbruster wrote: [...] >> My reason for four spaces is reducing churn. To see by how much, I >> redid your change. I found a few more notes that don't start with a >> capital letter, or don't end with a period. >> > > ^ Guess I'll re-audit for v2. Hang on to the list of cases you found. Happy to share my patch. > (Sorry for the churn, though. I obviously don't mind it as much as you do, > but I suspect I'm a lot less nimble with fiddling through git history than > you are and find the value of avoiding churn to be ... lower than you do, > in general. Respecting reviewer time is a strong argument, I apologize that > some non-mechanical changes snuck into the patch. The downside of hacking > together a very large series.) You did a good job splitting it up. Minor mistakes are bound to happen. Got to give the reviewer soemthing to find ;) [...]
[PATCH v4 0/3] Check clock connection between STM32L4x5 RCC and peripherals
Among implemented STM32L4x5 devices, USART, GPIO and SYSCFG have a clock source, but none has a corresponding test in QEMU. This patch makes sure that all 3 devices create a clock correctly, adds a QOM property to access clocks' periods from QTests, and adds QTests checking that clock enable in RCC has the expected results for all 3 devices. Thank you for the reviews. Changes from v3 to v4: - removed 2nd commit (it was bumping up version id in `vmstate_stm32l4x5_usart_base`, which is useless when not adding any fields), it was a misunderstanding - in `clock.c`, `vmstate_stm32l4x5_usart_base`, renamed `freq_hz` to `period` - in `clocks.rst`, specified that `qtest-clock-period` is only usable from the QTests and not QEMU - in `qtest/stm32l4x5.h`, used macros from "clock.h" to compute the expected clock period in the right unit - in `qtest/stm32l4x5.h`, removed "osdep.h" include Changes from "v1" to v3: - adding a commit to expose `qtest-clock-period`, a QOM property for all clocks, only accessible from QTests, and mention it in clock.rst - adapt QTests so that they use clock period instead of clock frequency - remove `clock-freq-hz` QOM property in STM32L4x5 USART and SYSCFG - dropping the commit migrating GPIO clocks as it's already upstream Changes from v1 to an unfortunate second "v1": - upgrading `VMStateDescription` to version 2 to account for `VMSTATE_CLOCK()` - QTests : consolidating `get_clock_freq_hz()` in a header and making appropriate changes in stm32l4x5q_*-test.c Signed-off-by: Inès Varhol Inès Varhol (3): hw/misc: Create STM32L4x5 SYSCFG clock hw/clock: Expose 'qtest-clock-period' QOM property for QTests tests/qtest: Check STM32L4x5 clock connections docs/devel/clocks.rst | 6 + include/hw/misc/stm32l4x5_syscfg.h | 1 + tests/qtest/stm32l4x5.h | 42 + hw/arm/stm32l4x5_soc.c | 2 ++ hw/core/clock.c | 16 +++ hw/misc/stm32l4x5_syscfg.c | 19 +++-- tests/qtest/stm32l4x5_gpio-test.c | 23 tests/qtest/stm32l4x5_syscfg-test.c | 20 -- tests/qtest/stm32l4x5_usart-test.c | 26 ++ 9 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 tests/qtest/stm32l4x5.h -- 2.43.2
[PATCH v4 1/3] hw/misc: Create STM32L4x5 SYSCFG clock
This commit creates a clock in STM32L4x5 SYSCFG and wires it up to the
corresponding clock from STM32L4x5 RCC.
Signed-off-by: Inès Varhol
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
---
include/hw/misc/stm32l4x5_syscfg.h | 1 +
hw/arm/stm32l4x5_soc.c | 2 ++
hw/misc/stm32l4x5_syscfg.c | 19 +--
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/hw/misc/stm32l4x5_syscfg.h
b/include/hw/misc/stm32l4x5_syscfg.h
index 23bb564150..c450df2b9e 100644
--- a/include/hw/misc/stm32l4x5_syscfg.h
+++ b/include/hw/misc/stm32l4x5_syscfg.h
@@ -48,6 +48,7 @@ struct Stm32l4x5SyscfgState {
uint32_t swpr2;
qemu_irq gpio_out[GPIO_NUM_PINS];
+Clock *clk;
};
#endif
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 38f7a2d5d9..fb2afa6cfe 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -236,6 +236,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
Error **errp)
/* System configuration controller */
busdev = SYS_BUS_DEVICE(&s->syscfg);
+qdev_connect_clock_in(DEVICE(&s->syscfg), "clk",
+qdev_get_clock_out(DEVICE(&(s->rcc)), "syscfg-out"));
if (!sysbus_realize(busdev, errp)) {
return;
}
diff --git a/hw/misc/stm32l4x5_syscfg.c b/hw/misc/stm32l4x5_syscfg.c
index a5a1ce2680..a947a9e036 100644
--- a/hw/misc/stm32l4x5_syscfg.c
+++ b/hw/misc/stm32l4x5_syscfg.c
@@ -26,6 +26,9 @@
#include "trace.h"
#include "hw/irq.h"
#include "migration/vmstate.h"
+#include "hw/clock.h"
+#include "hw/qdev-clock.h"
+#include "qapi/error.h"
#include "hw/misc/stm32l4x5_syscfg.h"
#include "hw/gpio/stm32l4x5_gpio.h"
@@ -225,12 +228,22 @@ static void stm32l4x5_syscfg_init(Object *obj)
qdev_init_gpio_in(DEVICE(obj), stm32l4x5_syscfg_set_irq,
GPIO_NUM_PINS * NUM_GPIOS);
qdev_init_gpio_out(DEVICE(obj), s->gpio_out, GPIO_NUM_PINS);
+s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
+}
+
+static void stm32l4x5_syscfg_realize(DeviceState *dev, Error **errp)
+{
+Stm32l4x5SyscfgState *s = STM32L4X5_SYSCFG(dev);
+if (!clock_has_source(s->clk)) {
+error_setg(errp, "SYSCFG: clk input must be connected");
+return;
+}
}
static const VMStateDescription vmstate_stm32l4x5_syscfg = {
.name = TYPE_STM32L4X5_SYSCFG,
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
.fields = (VMStateField[]) {
VMSTATE_UINT32(memrmp, Stm32l4x5SyscfgState),
VMSTATE_UINT32(cfgr1, Stm32l4x5SyscfgState),
@@ -241,6 +254,7 @@ static const VMStateDescription vmstate_stm32l4x5_syscfg = {
VMSTATE_UINT32(swpr, Stm32l4x5SyscfgState),
VMSTATE_UINT32(skr, Stm32l4x5SyscfgState),
VMSTATE_UINT32(swpr2, Stm32l4x5SyscfgState),
+VMSTATE_CLOCK(clk, Stm32l4x5SyscfgState),
VMSTATE_END_OF_LIST()
}
};
@@ -251,6 +265,7 @@ static void stm32l4x5_syscfg_class_init(ObjectClass *klass,
void *data)
ResettableClass *rc = RESETTABLE_CLASS(klass);
dc->vmsd = &vmstate_stm32l4x5_syscfg;
+dc->realize = stm32l4x5_syscfg_realize;
rc->phases.hold = stm32l4x5_syscfg_hold_reset;
}
--
2.43.2
[PATCH v4 3/3] tests/qtest: Check STM32L4x5 clock connections
For USART, GPIO and SYSCFG devices, check that clock frequency before
and after enabling the peripheral clock in RCC is correct.
Signed-off-by: Inès Varhol
Reviewed-by: Peter Maydell
---
tests/qtest/stm32l4x5.h | 42 +
tests/qtest/stm32l4x5_gpio-test.c | 23
tests/qtest/stm32l4x5_syscfg-test.c | 20 --
tests/qtest/stm32l4x5_usart-test.c | 26 ++
4 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 tests/qtest/stm32l4x5.h
diff --git a/tests/qtest/stm32l4x5.h b/tests/qtest/stm32l4x5.h
new file mode 100644
index 00..2d21cc666c
--- /dev/null
+++ b/tests/qtest/stm32l4x5.h
@@ -0,0 +1,42 @@
+/*
+ * QTest testcase header for STM32L4X5 :
+ * used for consolidating common objects in stm32l4x5_*-test.c
+ *
+ * Copyright (c) 2024 Arnaud Minier
+ * Copyright (c) 2024 Inès Varhol
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "libqtest.h"
+
+/* copied from clock.h */
+#define CLOCK_PERIOD_1SEC (10llu << 32)
+#define CLOCK_PERIOD_FROM_HZ(hz) (((hz) != 0) ? CLOCK_PERIOD_1SEC / (hz) : 0u)
+/*
+ * MSI (4 MHz) is used as system clock source after startup
+ * from Reset.
+ * AHB, APB1 and APB2 prescalers are set to 1 at reset.
+ */
+#define SYSCLK_PERIOD CLOCK_PERIOD_FROM_HZ(400)
+#define RCC_AHB2ENR 0x4002104C
+#define RCC_APB1ENR1 0x40021058
+#define RCC_APB1ENR2 0x4002105C
+#define RCC_APB2ENR 0x40021060
+
+
+static inline uint64_t get_clock_period(QTestState *qts, const char *path)
+{
+uint64_t clock_period = 0;
+QDict *r;
+
+r = qtest_qmp(qts, "{ 'execute': 'qom-get', 'arguments':"
+" { 'path': %s, 'property': 'qtest-clock-period'} }", path);
+g_assert_false(qdict_haskey(r, "error"));
+clock_period = qdict_get_int(r, "return");
+qobject_unref(r);
+return clock_period;
+}
+
+
diff --git a/tests/qtest/stm32l4x5_gpio-test.c
b/tests/qtest/stm32l4x5_gpio-test.c
index 72a7823406..c0686c7b30 100644
--- a/tests/qtest/stm32l4x5_gpio-test.c
+++ b/tests/qtest/stm32l4x5_gpio-test.c
@@ -10,6 +10,7 @@
#include "qemu/osdep.h"
#include "libqtest-single.h"
+#include "stm32l4x5.h"
#define GPIO_BASE_ADDR 0x4800
#define GPIO_SIZE 0x400
@@ -505,6 +506,26 @@ static void test_bsrr_brr(const void *data)
gpio_writel(gpio, ODR, reset(gpio, ODR));
}
+static void test_clock_enable(void)
+{
+/*
+ * For each GPIO, enable its clock in RCC
+ * and check that its clock period changes to SYSCLK_PERIOD
+ */
+unsigned int gpio_id;
+
+for (uint32_t gpio = GPIO_A; gpio <= GPIO_H; gpio += GPIO_B - GPIO_A) {
+gpio_id = get_gpio_id(gpio);
+g_autofree char *path = g_strdup_printf("/machine/soc/gpio%c/clk",
+gpio_id + 'a');
+g_assert_cmpuint(get_clock_period(global_qtest, path), ==, 0);
+/* Enable the gpio clock */
+writel(RCC_AHB2ENR, readl(RCC_AHB2ENR) | (0x1 << gpio_id));
+g_assert_cmpuint(get_clock_period(global_qtest, path), ==,
+ SYSCLK_PERIOD);
+}
+}
+
int main(int argc, char **argv)
{
int ret;
@@ -556,6 +577,8 @@ int main(int argc, char **argv)
qtest_add_data_func("stm32l4x5/gpio/test_bsrr_brr2",
test_data(GPIO_D, 0),
test_bsrr_brr);
+qtest_add_func("stm32l4x5/gpio/test_clock_enable",
+ test_clock_enable);
qtest_start("-machine b-l475e-iot01a");
ret = g_test_run();
diff --git a/tests/qtest/stm32l4x5_syscfg-test.c
b/tests/qtest/stm32l4x5_syscfg-test.c
index 506ca08bc2..8eaffe43ea 100644
--- a/tests/qtest/stm32l4x5_syscfg-test.c
+++ b/tests/qtest/stm32l4x5_syscfg-test.c
@@ -10,6 +10,7 @@
#include "qemu/osdep.h"
#include "libqtest-single.h"
+#include "stm32l4x5.h"
#define SYSCFG_BASE_ADDR 0x4001
#define SYSCFG_MEMRMP 0x00
@@ -26,7 +27,9 @@
#define INVALID_ADDR 0x2C
/* SoC forwards GPIOs to SysCfg */
-#define SYSCFG "/machine/soc"
+#define SOC "/machine/soc"
+#define SYSCFG "/machine/soc/syscfg"
+#define SYSCFG_CLK "/machine/soc/syscfg/clk"
#define EXTI "/machine/soc/exti"
static void syscfg_writel(unsigned int offset, uint32_t value)
@@ -41,7 +44,7 @@ static uint32_t syscfg_readl(unsigned int offset)
static void syscfg_set_irq(int num, int level)
{
- qtest_set_irq_in(global_qtest, SYSCFG, NULL, num, level);
+ qtest_set_irq_in(global_qtest, SOC, NULL, num, level);
}
static void system_reset(void)
@@ -301,6 +304,17 @@ static void test_irq_gpio_multiplexer(void)
syscfg_writel(SYSCFG_EXTICR1, 0x);
}
+static void test_clock_enable(void)
+{
+g_assert_cmpuint(get_clock_period(global_qtest, SYSCFG_CLK), ==, 0);
+
+/* Enable SYSCFG clock */
+writel(RCC_APB2ENR, readl(RCC_APB2ENR) | (0x1 << 0));
+
+g_assert_cmpuint(get_clock_period(global_qtest, SYSCFG_CLK), ==,
+
[PATCH v4 2/3] hw/clock: Expose 'qtest-clock-period' QOM property for QTests
Expose the clock period via the QOM 'qtest-clock-period' property so it
can be used in QTests. This property is only accessible in QTests (not
via HMP).
Signed-off-by: Inès Varhol
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Luc Michel
---
docs/devel/clocks.rst | 6 ++
hw/core/clock.c | 16
2 files changed, 22 insertions(+)
diff --git a/docs/devel/clocks.rst b/docs/devel/clocks.rst
index 177ee1c90d..3f744f2be1 100644
--- a/docs/devel/clocks.rst
+++ b/docs/devel/clocks.rst
@@ -358,6 +358,12 @@ humans (for instance in debugging), use
``clock_display_freq()``,
which returns a prettified string-representation, e.g. "33.3 MHz".
The caller must free the string with g_free() after use.
+It's also possible to retrieve the clock period from a QTest by
+accessing QOM property ``qtest-clock-period`` using a QMP command.
+This property is only present when the device is being run under
+the ``qtest`` accelerator; it is not available when QEMU is
+being run normally.
+
Calculating expiry deadlines
diff --git a/hw/core/clock.c b/hw/core/clock.c
index e212865307..cbe7b1bc46 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -13,6 +13,8 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
+#include "qapi/visitor.h"
+#include "sysemu/qtest.h"
#include "hw/clock.h"
#include "trace.h"
@@ -158,6 +160,15 @@ bool clock_set_mul_div(Clock *clk, uint32_t multiplier,
uint32_t divider)
return true;
}
+static void clock_period_prop_get(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+Clock *clk = CLOCK(obj);
+uint64_t period = clock_get(clk);
+visit_type_uint64(v, name, &period, errp);
+}
+
+
static void clock_initfn(Object *obj)
{
Clock *clk = CLOCK(obj);
@@ -166,6 +177,11 @@ static void clock_initfn(Object *obj)
clk->divider = 1;
QLIST_INIT(&clk->children);
+
+if (qtest_enabled()) {
+object_property_add(obj, "qtest-clock-period", "uint64",
+clock_period_prop_get, NULL, NULL, NULL);
+}
}
static void clock_finalizefn(Object *obj)
--
2.43.2
[PULL 01/18] hw/net/can/xlnx-versal-canfd: Fix sorting of the tx queue
From: Shiva sagar Myana
Returning an uint32_t casted to a gint from g_cmp_ids causes the tx queue to
become wrongly sorted when executing g_slist_sort. Fix this by always
returning -1 or 1 from g_cmp_ids based on the ID comparison instead.
Also, if two message IDs are the same, sort them by using their index and
transmit the message at the lowest index first.
Signed-off-by: Shiva sagar Myana
Reviewed-by: Francisco Iglesias
Message-id: 20240603051732.3334571-1-shivasagar.my...@amd.com
Signed-off-by: Peter Maydell
---
hw/net/can/xlnx-versal-canfd.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/net/can/xlnx-versal-canfd.c b/hw/net/can/xlnx-versal-canfd.c
index 47a14cfe633..5f083c21e93 100644
--- a/hw/net/can/xlnx-versal-canfd.c
+++ b/hw/net/can/xlnx-versal-canfd.c
@@ -1312,7 +1312,10 @@ static gint g_cmp_ids(gconstpointer data1, gconstpointer
data2)
tx_ready_reg_info *tx_reg_1 = (tx_ready_reg_info *) data1;
tx_ready_reg_info *tx_reg_2 = (tx_ready_reg_info *) data2;
-return tx_reg_1->can_id - tx_reg_2->can_id;
+if (tx_reg_1->can_id == tx_reg_2->can_id) {
+return (tx_reg_1->reg_num < tx_reg_2->reg_num) ? -1 : 1;
+}
+return (tx_reg_1->can_id < tx_reg_2->can_id) ? -1 : 1;
}
static void free_list(GSList *list)
--
2.34.1
[PULL 09/18] scripts/coverity-scan/COMPONENTS.md: Include libqmp in testlibs
Add libqmp to the testlibs component. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-6-peter.mayd...@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 3864f8eda07..858190be097 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -154,7 +154,7 @@ sysemu ~ .*/qemu(/include/.*) testlibs - ~ .*/qemu(/tests/qtest(/libqos/.*|/libqtest.*)) + ~ .*/qemu(/tests/qtest(/libqos/.*|/libqtest.*|/libqmp.*)) tests ~ .*/qemu(/tests/.*) -- 2.34.1
[PULL 15/18] hw/arm/virt: Avoid unexpected warning from Linux guest on host with Fujitsu CPUs
From: Zhenyu Zhang
Multiple warning messages and corresponding backtraces are observed when Linux
guest is booted on the host with Fujitsu CPUs. One of them is shown as below.
[0.032443] [ cut here ]
[0.032446] uart-pl011 900.pl011: ARCH_DMA_MINALIGN smaller than
CTR_EL0.CWG (128 < 256)
[0.032454] WARNING: CPU: 0 PID: 1 at arch/arm64/mm/dma-mapping.c:54
arch_setup_dma_ops+0xbc/0xcc
[0.032470] Modules linked in:
[0.032475] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.14.0-452.el9.aarch64
[0.032481] Hardware name: linux,dummy-virt (DT)
[0.032484] pstate: 6045 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[0.032490] pc : arch_setup_dma_ops+0xbc/0xcc
[0.032496] lr : arch_setup_dma_ops+0xbc/0xcc
[0.032501] sp : 80008003b860
[0.032503] x29: 80008003b860 x28: x27: aae4b949049c
[0.032510] x26: x25: x24:
[0.032517] x23: 0100 x22: x21:
[0.032523] x20: 0001 x19: 2f06c02ea400 x18:
[0.032529] x17: 208a5f76 x16: 6589dbcb x15: aae4ba071c89
[0.032535] x14: x13: aae4ba071c84 x12: 455f525443206e61
[0.032541] x11: 68742072656c6c61 x10: 0029 x9 : aae4b7d21da4
[0.032547] x8 : 0029 x7 : 4c414e494d5f414d x6 : 0029
[0.032553] x5 : 000f x4 : aae4b9617a00 x3 : 0001
[0.032558] x2 : x1 : x0 : 2f06c029be40
[0.032564] Call trace:
[0.032566] arch_setup_dma_ops+0xbc/0xcc
[0.032572] of_dma_configure_id+0x138/0x300
[0.032591] amba_dma_configure+0x34/0xc0
[0.032600] really_probe+0x78/0x3dc
[0.032614] __driver_probe_device+0x108/0x160
[0.032619] driver_probe_device+0x44/0x114
[0.032624] __device_attach_driver+0xb8/0x14c
[0.032629] bus_for_each_drv+0x88/0xe4
[0.032634] __device_attach+0xb0/0x1e0
[0.032638] device_initial_probe+0x18/0x20
[0.032643] bus_probe_device+0xa8/0xb0
[0.032648] device_add+0x4b4/0x6c0
[0.032652] amba_device_try_add.part.0+0x48/0x360
[0.032657] amba_device_add+0x104/0x144
[0.032662] of_amba_device_create.isra.0+0x100/0x1c4
[0.032666] of_platform_bus_create+0x294/0x35c
[0.032669] of_platform_populate+0x5c/0x150
[0.032672] of_platform_default_populate_init+0xd0/0xec
[0.032697] do_one_initcall+0x4c/0x2e0
[0.032701] do_initcalls+0x100/0x13c
[0.032707] kernel_init_freeable+0x1c8/0x21c
[0.032712] kernel_init+0x28/0x140
[0.032731] ret_from_fork+0x10/0x20
[0.032735] ---[ end trace ]---
In Linux, a check is applied to every device which is exposed through
device-tree node. The warning message is raised when the device isn't
DMA coherent and the cache line size is larger than ARCH_DMA_MINALIGN
(128 bytes). The cache line is sorted from CTR_EL0[CWG], which corresponds
to 256 bytes on the guest CPUs. The DMA coherent capability is claimed
through 'dma-coherent' in their device-tree nodes or parent nodes.
This happens even when the device doesn't implement or use DMA at all,
for legacy reasons.
Fix the issue by adding 'dma-coherent' property to the device-tree root
node, meaning all devices are capable of DMA coherent by default.
This both suppresses the spurious kernel warnings and also guards
against possible future QEMU bugs where we add a DMA-capable device
and forget to mark it as dma-coherent.
Signed-off-by: Zhenyu Zhang
Reviewed-by: Gavin Shan
Reviewed-by: Donald Dutile
Message-id: 20240612020506.307793-1-zheny...@redhat.com
[PMM: tweaked commit message]
Signed-off-by: Peter Maydell
---
hw/arm/virt.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 85556152563..0784ee7f466 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -271,6 +271,17 @@ static void create_fdt(VirtMachineState *vms)
qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
qemu_fdt_setprop_string(fdt, "/", "model", "linux,dummy-virt");
+/*
+ * For QEMU, all DMA is coherent. Advertising this in the root node
+ * has two benefits:
+ *
+ * - It avoids potential bugs where we forget to mark a DMA
+ * capable device as being dma-coherent
+ * - It avoids spurious warnings from the Linux kernel about
+ * devices which can't do DMA at all
+ */
+qemu_fdt_setprop(fdt, "/", "dma-coherent", NULL, 0);
+
/* /chosen must exist for load_dtb to fill in necessary properties later */
qemu_fdt_add_subnode(fdt, "/chosen");
if (vms->dtb_randomness) {
--
2.34.1
[PULL 11/18] hw/usb/hcd-dwc2: Handle invalid address access in read and write functions
From: Zheyu Ma This commit modifies the dwc2_hsotg_read() and dwc2_hsotg_write() functions to handle invalid address access gracefully. Instead of using g_assert_not_reached(), which causes the program to abort, the functions now log an error message and return a default value for reads or do nothing for writes. This change prevents the program from aborting and provides clear log messages indicating when an invalid memory address is accessed. Reproducer: cat << EOF | qemu-system-aarch64 -display none \ -machine accel=qtest, -m 512M -machine raspi2b -m 1G -nodefaults \ -usb -drive file=null-co://,if=none,format=raw,id=disk0 -device \ usb-storage,port=1,drive=disk0 -qtest stdio readl 0x3f980dfb EOF Signed-off-by: Zheyu Ma Reviewed-by: Paul Zimmerman Message-id: 20240618135610.3109175-1-zheyum...@gmail.com Signed-off-by: Peter Maydell --- hw/usb/hcd-dwc2.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c index 8cac9c0a062..b4f0652c7d2 100644 --- a/hw/usb/hcd-dwc2.c +++ b/hw/usb/hcd-dwc2.c @@ -1128,7 +1128,10 @@ static uint64_t dwc2_hsotg_read(void *ptr, hwaddr addr, unsigned size) val = dwc2_pcgreg_read(ptr, addr, (addr - HSOTG_REG(0xe00)) >> 2, size); break; default: -g_assert_not_reached(); +qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n", + __func__, addr); +val = 0; +break; } return val; @@ -1160,7 +1163,9 @@ static void dwc2_hsotg_write(void *ptr, hwaddr addr, uint64_t val, dwc2_pcgreg_write(ptr, addr, (addr - HSOTG_REG(0xe00)) >> 2, val, size); break; default: -g_assert_not_reached(); +qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"HWADDR_PRIx"\n", + __func__, addr); +break; } } -- 2.34.1
[PULL 10/18] hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu
From: Zheyu Ma
This commit updates the a9_gtimer_get_current_cpu() function to handle
cases where QTest is enabled. When QTest is used, it returns 0 instead
of dereferencing the current_cpu, which can be NULL. This prevents the
program from crashing during QTest runs.
Reproducer:
cat << EOF | qemu-system-aarch64 -display \
none -machine accel=qtest, -m 512M -machine npcm750-evb -qtest stdio
writel 0xf03fe20c 0x26d7468c
EOF
Signed-off-by: Zheyu Ma
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240618144009.3137806-1-zheyum...@gmail.com
Signed-off-by: Peter Maydell
---
hw/timer/a9gtimer.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
index a2ac5bdfb99..64d80cdf6a3 100644
--- a/hw/timer/a9gtimer.c
+++ b/hw/timer/a9gtimer.c
@@ -32,6 +32,7 @@
#include "qemu/log.h"
#include "qemu/module.h"
#include "hw/core/cpu.h"
+#include "sysemu/qtest.h"
#ifndef A9_GTIMER_ERR_DEBUG
#define A9_GTIMER_ERR_DEBUG 0
@@ -48,6 +49,10 @@
static inline int a9_gtimer_get_current_cpu(A9GTimerState *s)
{
+if (qtest_enabled()) {
+return 0;
+}
+
if (current_cpu->cpu_index >= s->num_cpu) {
hw_error("a9gtimer: num-cpu %d but this cpu is %d!\n",
s->num_cpu, current_cpu->cpu_index);
--
2.34.1
[PULL 05/18] scripts/coverity-scan/COMPONENTS.md: Update paths to match gitlab CI
Since commit 83aa1baa069c we have been running the build for Coverity Scan as a Gitlab CI job, rather than the old setup where it was run on a local developer's machine. This is working well, but the absolute paths of files are different for the Gitlab CI job, which means that the regexes we use to identify Coverity components no longer work. With Gitlab CI builds the file paths are of the form /builds/qemu-project/qemu/accel/kvm/kvm-all.c rather than the old /qemu/accel/kvm/kvm-all.c and our regexes all don't match. Update all the regexes to start with .*/qemu/ . This will hopefully avoid the need to change them again in future if the build path changes again. This change was made with a search-and-replace of (/qemu)? to .*/qemu . Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-2-peter.mayd...@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 104 ++-- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 1537e49cd5a..98d4bcd6a50 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -1,157 +1,157 @@ This is the list of currently configured Coverity components: alpha - ~ (/qemu)?((/include)?/hw/alpha/.*|/target/alpha/.*) + ~ .*/qemu((/include)?/hw/alpha/.*|/target/alpha/.*) arm - ~ (/qemu)?((/include)?/hw/arm/.*|(/include)?/hw/.*/(arm|allwinner-a10|bcm28|digic|exynos|imx|omap|stellaris|pxa2xx|versatile|zynq|cadence).*|/hw/net/xgmac.c|/hw/ssi/xilinx_spips.c|/target/arm/.*) + ~ .*/qemu((/include)?/hw/arm/.*|(/include)?/hw/.*/(arm|allwinner-a10|bcm28|digic|exynos|imx|omap|stellaris|pxa2xx|versatile|zynq|cadence).*|/hw/net/xgmac.c|/hw/ssi/xilinx_spips.c|/target/arm/.*) avr - ~ (/qemu)?((/include)?/hw/avr/.*|/target/avr/.*) + ~ .*/qemu((/include)?/hw/avr/.*|/target/avr/.*) cris - ~ (/qemu)?((/include)?/hw/cris/.*|/target/cris/.*) + ~ .*/qemu((/include)?/hw/cris/.*|/target/cris/.*) hexagon-gen (component should be ignored in analysis) - ~ (/qemu)?(/target/hexagon/.*generated.*) + ~ .*/qemu(/target/hexagon/.*generated.*) hexagon - ~ (/qemu)?(/target/hexagon/.*) + ~ .*/qemu(/target/hexagon/.*) hppa - ~ (/qemu)?((/include)?/hw/hppa/.*|/target/hppa/.*) + ~ .*/qemu((/include)?/hw/hppa/.*|/target/hppa/.*) i386 - ~ (/qemu)?((/include)?/hw/i386/.*|/target/i386/.*|/hw/intc/[^/]*apic[^/]*\.c) + ~ .*/qemu((/include)?/hw/i386/.*|/target/i386/.*|/hw/intc/[^/]*apic[^/]*\.c) loongarch - ~ (/qemu)?((/include)?/hw/(loongarch/.*|.*/loongarch.*)|/target/loongarch/.*) + ~ .*/qemu((/include)?/hw/(loongarch/.*|.*/loongarch.*)|/target/loongarch/.*) m68k - ~ (/qemu)?((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*|(/include)?/hw/nubus/.*) + ~ .*/qemu((/include)?/hw/m68k/.*|/target/m68k/.*|(/include)?/hw(/.*)?/mcf.*|(/include)?/hw/nubus/.*) microblaze - ~ (/qemu)?((/include)?/hw/microblaze/.*|/target/microblaze/.*) + ~ .*/qemu((/include)?/hw/microblaze/.*|/target/microblaze/.*) mips - ~ (/qemu)?((/include)?/hw/mips/.*|/target/mips/.*) + ~ .*/qemu((/include)?/hw/mips/.*|/target/mips/.*) openrisc - ~ (/qemu)?((/include)?/hw/openrisc/.*|/target/openrisc/.*) + ~ .*/qemu((/include)?/hw/openrisc/.*|/target/openrisc/.*) ppc - ~ (/qemu)?((/include)?/hw/ppc/.*|/target/ppc/.*|/hw/pci-host/(uninorth.*|dec.*|prep.*|ppc.*)|/hw/misc/macio/.*|(/include)?/hw/.*/(xics|openpic|spapr).*) + ~ .*/qemu((/include)?/hw/ppc/.*|/target/ppc/.*|/hw/pci-host/(uninorth.*|dec.*|prep.*|ppc.*)|/hw/misc/macio/.*|(/include)?/hw/.*/(xics|openpic|spapr).*) riscv - ~ (/qemu)?((/include)?/hw/riscv/.*|/target/riscv/.*|/hw/.*/(riscv_|ibex_|sifive_).*) + ~ .*/qemu((/include)?/hw/riscv/.*|/target/riscv/.*|/hw/.*/(riscv_|ibex_|sifive_).*) rx - ~ (/qemu)?((/include)?/hw/rx/.*|/target/rx/.*) + ~ .*/qemu((/include)?/hw/rx/.*|/target/rx/.*) s390 - ~ (/qemu)?((/include)?/hw/s390x/.*|/target/s390x/.*|/hw/.*/s390_.*) + ~ .*/qemu((/include)?/hw/s390x/.*|/target/s390x/.*|/hw/.*/s390_.*) sh4 - ~ (/qemu)?((/include)?/hw/sh4/.*|/target/sh4/.*) + ~ .*/qemu((/include)?/hw/sh4/.*|/target/sh4/.*) sparc - ~ (/qemu)?((/include)?/hw/sparc(64)?.*|/target/sparc/.*|/hw/.*/grlib.*|/hw/display/cg3.c) + ~ .*/qemu((/include)?/hw/sparc(64)?.*|/target/sparc/.*|/hw/.*/grlib.*|/hw/display/cg3.c) tricore - ~ (/qemu)?((/include)?/hw/tricore/.*|/target/tricore/.*) + ~ .*/qemu((/include)?/hw/tricore/.*|/target/tricore/.*) xtensa - ~ (/qemu)?((/include)?/hw/xtensa/.*|/target/xtensa/.*) + ~ .*/qemu((/include)?/hw/xtensa/.*|/target/xtensa/.*) 9pfs - ~ (/qemu)?(/hw/9pfs/.*|/fsdev/.*) + ~ .*/qemu(/hw/9pfs/.*|/fsdev/.*) audio - ~ (/qemu)?((/include)?/(audio|hw/audio)/.*) + ~ .*/qemu((/include)?/(audio|hw/audio)/.*) block - ~ (/qemu)?(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-p
[PULL 16/18] hw/misc: Set valid access size for Exynos4210 RNG
From: Zheyu Ma
The Exynos4210 RNG module requires 32-bit (4-byte) accesses to its registers.
According to the User Manual Section 25.3[1], the registers for RNG operations
are 32-bit. This change ensures that the memory region operations for the
RNG module enforce the correct access sizes, preventing invalid memory accesses.
[1]
http://www.mediafire.com/view/8ly2fqls3c9c31c/Exynos_4412_SCP_Users_Manual_Ver.0.10.00_Preliminary0.pdf
Reproducer:
cat << EOF | qemu-system-aarch64 -display none \
-machine accel=qtest, -m 512M -machine smdkc210 -qtest stdio
readb 0x10830454
EOF
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Zheyu Ma
Message-id: 20240618163701.3204975-1-zheyum...@gmail.com
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/misc/exynos4210_rng.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/misc/exynos4210_rng.c b/hw/misc/exynos4210_rng.c
index 0756bd32059..674d8eece5f 100644
--- a/hw/misc/exynos4210_rng.c
+++ b/hw/misc/exynos4210_rng.c
@@ -217,6 +217,8 @@ static const MemoryRegionOps exynos4210_rng_ops = {
.read = exynos4210_rng_read,
.write = exynos4210_rng_write,
.endianness = DEVICE_NATIVE_ENDIAN,
+.valid.min_access_size = 4,
+.valid.max_access_size = 4,
};
static void exynos4210_rng_reset(DeviceState *dev)
--
2.34.1
[PULL 02/18] hw/arm/sbsa-ref: switch to 1GHz timer frequency
From: Marcin Juszkiewicz
Updated firmware for QEMU CI is already in merge queue so we can move
platform to be future proof.
All supported cpus work fine with 1GHz timer frequency when firmware is
fresh enough.
Signed-off-by: Marcin Juszkiewicz
Reviewed-by: Leif Lindholm
Message-id: 20240531093729.220758-2-marcin.juszkiew...@linaro.org
Signed-off-by: Peter Maydell
---
hw/arm/sbsa-ref.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index e884692f07f..87884400e30 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -62,16 +62,12 @@
/*
* Generic timer frequency in Hz (which drives both the CPU generic timers
- * and the SBSA watchdog-timer). Older versions of the TF-A firmware
- * typically used with sbsa-ref (including the binaries in our Avocado test
- * Aarch64SbsarefMachine.test_sbsaref_alpine_linux_max_pauth_impdef
- * assume it is this value.
+ * and the SBSA watchdog-timer). Older (<2.11) versions of the TF-A firmware
+ * assumed 62.5MHz here.
*
- * TODO: this value is not architecturally correct for an Armv8.6 or
- * better CPU, so we should move to 1GHz once the TF-A fix above has
- * made it into a release and into our Avocado test.
+ * Starting with Armv8.6 CPU 1GHz timer frequency is mandated.
*/
-#define SBSA_GTIMER_HZ 6250
+#define SBSA_GTIMER_HZ 10
enum {
SBSA_FLASH,
--
2.34.1
[PULL 12/18] hw/arm/virt: Add serial aliases in DTB
If there is more than one UART in the DTB, then there is no guarantee
on which order a guest is supposed to initialise them. The standard
solution to this is "serialN" entries in the "/aliases" node of the
dtb which give the nodename of the UARTs.
At the moment we only have two UARTs in the DTB when one is for
the Secure world and one for the Non-Secure world, so this isn't
really a problem. However if we want to add a second NS UART we'll
need the aliases to ensure guests pick the right one.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240610162343.2131524-2-peter.mayd...@linaro.org
---
hw/arm/virt.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c7a1f754e72..61a9d47c026 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -284,6 +284,8 @@ static void create_fdt(VirtMachineState *vms)
}
}
+qemu_fdt_add_subnode(fdt, "/aliases");
+
/* Clock node, for the benefit of the UART. The kernel device tree
* binding documentation claims the PL011 node clock properties are
* optional but in practice if you omit them the kernel refuses to
@@ -939,7 +941,9 @@ static void create_uart(const VirtMachineState *vms, int
uart,
if (uart == VIRT_UART) {
qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
+qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename);
} else {
+qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial1", nodename);
/* Mark as not usable by the normal world */
qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled");
qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay");
--
2.34.1
[PULL 14/18] hw/arm/virt: allow creation of a second NonSecure UART
For some use-cases, it is helpful to have more than one UART
available to the guest. If the second UART slot is not already used
for a TrustZone Secure-World-only UART, create it as a NonSecure UART
only when the user provides a serial backend (e.g. via a second
-serial command line option).
This avoids problems where existing guest software only expects a
single UART, and gets confused by the second UART in the DTB. The
major example of this is older EDK2 firmware, which will send the
GRUB bootloader output to UART1 and the guest serial output to UART0.
Users who want to use both UARTs with a guest setup including EDK2
are advised to update to EDK2 release edk2-stable202311 or newer.
(The prebuilt EDK2 blobs QEMU upstream provides are new enough.)
The relevant EDK2 changes are the ones described here:
https://bugzilla.tianocore.org/show_bug.cgi?id=4577
Inspired-by: Axel Heider
Signed-off-by: Peter Maydell
Tested-by: Laszlo Ersek
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240610162343.2131524-4-peter.mayd...@linaro.org
---
docs/system/arm/virt.rst | 6 +-
include/hw/arm/virt.h| 1 +
hw/arm/virt-acpi-build.c | 12
hw/arm/virt.c| 38 +++---
4 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 26fcba00b76..e67e7f0f7c5 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -26,7 +26,7 @@ The virt board supports:
- PCI/PCIe devices
- Flash memory
-- One PL011 UART
+- Either one or two PL011 UARTs for the NonSecure World
- An RTC
- The fw_cfg device that allows a guest to obtain data from QEMU
- A PL061 GPIO controller
@@ -48,6 +48,10 @@ The virt board supports:
- A secure flash memory
- 16MB of secure RAM
+The second NonSecure UART only exists if a backend is configured
+explicitly (e.g. with a second -serial command line option) and
+TrustZone emulation is not enabled.
+
Supported guest CPU types:
- ``cortex-a7`` (32-bit)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 1227e7f7f08..ab961bb6a9b 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -151,6 +151,7 @@ struct VirtMachineState {
bool ras;
bool mte;
bool dtb_randomness;
+bool second_ns_uart_present;
OnOffAuto acpi;
VirtGICType gic_version;
VirtIOMMUType iommu;
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index eb5796e309b..b2366f24f96 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -79,11 +79,11 @@ static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState
*vms)
}
static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
- uint32_t uart_irq)
+ uint32_t uart_irq, int uartidx)
{
-Aml *dev = aml_device("COM0");
+Aml *dev = aml_device("COM%d", uartidx);
aml_append(dev, aml_name_decl("_HID", aml_string("ARMH0011")));
-aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+aml_append(dev, aml_name_decl("_UID", aml_int(uartidx)));
Aml *crs = aml_resource_template();
aml_append(crs, aml_memory32_fixed(uart_memmap->base,
@@ -817,7 +817,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
scope = aml_scope("\\_SB");
acpi_dsdt_add_cpus(scope, vms);
acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0],
- (irqmap[VIRT_UART0] + ARM_SPI_BASE));
+ (irqmap[VIRT_UART0] + ARM_SPI_BASE), 0);
+if (vms->second_ns_uart_present) {
+acpi_dsdt_add_uart(scope, &memmap[VIRT_UART1],
+ (irqmap[VIRT_UART1] + ARM_SPI_BASE), 1);
+}
if (vmc->acpi_expose_flash) {
acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ffb4983885f..85556152563 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -906,7 +906,7 @@ static void create_gic(VirtMachineState *vms, MemoryRegion
*mem)
}
static void create_uart(const VirtMachineState *vms, int uart,
-MemoryRegion *mem, Chardev *chr)
+MemoryRegion *mem, Chardev *chr, bool secure)
{
char *nodename;
hwaddr base = vms->memmap[uart].base;
@@ -944,6 +944,8 @@ static void create_uart(const VirtMachineState *vms, int
uart,
qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename);
} else {
qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial1", nodename);
+}
+if (secure) {
/* Mark as not usable by the normal world */
qemu_fdt_setprop_string(ms->fdt, nodename, "status", "disabled");
qemu_fdt_setprop_string(ms->fdt, nodename, "secure-status", "okay");
@@ -2317,11 +2319,41 @@ static void machvirt_init(MachineState *machine)
fdt_add_pmu_nodes(vms);
-create_uart(vms, VIRT_UART0, sysmem, serial_hd(0));
+/*
+ * The fir
[PULL 18/18] hw/arm/sbsa-ref: Enable CPU cluster on ARM sbsa machine
From: Xiong Yining
Enable CPU cluster support on SbsaQemu platform, so that users can
specify a 4-level CPU hierarchy sockets/clusters/cores/threads. And
this topology can be passed to the firmware through /cpus/topology
Device Tree.
Signed-off-by: Xiong Yining
Reviewed-by: Marcin Juszkiewicz
Reviewed-by: Leif Lindholm
Message-id: 20240607103825.1295328-2-xiongyining1...@phytium.com.cn
Tested-by: Marcin Juszkiewicz
Signed-off-by: Peter Maydell
---
docs/system/arm/sbsa.rst | 4
hw/arm/sbsa-ref.c| 11 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/docs/system/arm/sbsa.rst b/docs/system/arm/sbsa.rst
index 2bf22a1d0b0..2bf3fc8d59d 100644
--- a/docs/system/arm/sbsa.rst
+++ b/docs/system/arm/sbsa.rst
@@ -62,6 +62,7 @@ The devicetree reports:
- platform version
- GIC addresses
- NUMA node id for CPUs and memory
+ - CPU topology information
Platform version
@@ -88,3 +89,6 @@ Platform version changes:
0.3
The USB controller is an XHCI device, not EHCI.
+
+0.4
+ CPU topology information is present in devicetree.
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 87884400e30..ae37a923015 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -219,7 +219,7 @@ static void create_fdt(SBSAMachineState *sms)
*fw compatibility.
*/
qemu_fdt_setprop_cell(fdt, "/", "machine-version-major", 0);
-qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 3);
+qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 4);
if (ms->numa_state->have_numa_distance) {
int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
@@ -276,6 +276,14 @@ static void create_fdt(SBSAMachineState *sms)
g_free(nodename);
}
+/* Add CPU topology description through fdt node topology. */
+qemu_fdt_add_subnode(sms->fdt, "/cpus/topology");
+
+qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "sockets",
ms->smp.sockets);
+qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "clusters",
ms->smp.clusters);
+qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "cores", ms->smp.cores);
+qemu_fdt_setprop_cell(sms->fdt, "/cpus/topology", "threads",
ms->smp.threads);
+
sbsa_fdt_add_gic_node(sms);
}
@@ -898,6 +906,7 @@ static void sbsa_ref_class_init(ObjectClass *oc, void *data)
mc->default_ram_size = 1 * GiB;
mc->default_ram_id = "sbsa-ref.ram";
mc->default_cpus = 4;
+mc->smp_props.clusters_supported = true;
mc->possible_cpu_arch_ids = sbsa_ref_possible_cpu_arch_ids;
mc->cpu_index_to_instance_props = sbsa_ref_cpu_index_to_props;
mc->get_default_cpu_node_id = sbsa_ref_get_default_cpu_node_id;
--
2.34.1
[PULL 03/18] hw/intc/arm_gic: Fix deactivation of SPI lines
From: "Edgar E. Iglesias"
Julien reported that he has seen strange behaviour when running
Xen on QEMU using GICv2. When Xen migrates a guest's vCPU from
one pCPU to another while the vCPU is handling an interrupt, the
guest is unable to properly deactivate interrupts.
Looking at it a little closer, our GICv2 model treats
deactivation of SPI lines as if they were PPI's, i.e banked per
CPU core. The state for active interrupts should only be banked
for PPI lines, not for SPI lines.
Make deactivation of SPI lines unbanked, similar to how we
handle writes to GICD_ICACTIVER.
Reported-by: Julien Grall
Signed-off-by: Edgar E. Iglesias
Message-id: 20240605143044.2029444-2-edgar.igles...@gmail.com
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/intc/gic_internal.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 8d29b40ca10..8ddbf554c69 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -280,6 +280,8 @@ static inline void gic_set_active(GICState *s, int irq, int
cpu)
static inline void gic_clear_active(GICState *s, int irq, int cpu)
{
+unsigned int cm;
+
if (gic_is_vcpu(cpu)) {
uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
GICH_LR_CLEAR_ACTIVE(*entry);
@@ -301,11 +303,13 @@ static inline void gic_clear_active(GICState *s, int irq,
int cpu)
* the GIC is secure.
*/
if (!s->security_extn || GIC_DIST_TEST_GROUP(phys_irq, 1 << rcpu))
{
-GIC_DIST_CLEAR_ACTIVE(phys_irq, 1 << rcpu);
+cm = phys_irq < GIC_INTERNAL ? 1 << rcpu : ALL_CPU_MASK;
+GIC_DIST_CLEAR_ACTIVE(phys_irq, cm);
}
}
} else {
-GIC_DIST_CLEAR_ACTIVE(irq, 1 << cpu);
+cm = irq < GIC_INTERNAL ? 1 << cpu : ALL_CPU_MASK;
+GIC_DIST_CLEAR_ACTIVE(irq, cm);
}
}
--
2.34.1
[PULL 07/18] scripts/coverity-scan/COMPONENTS.md: Add crypto headers in host/include to the crypto component
host/include/*/host/crypto/ are relatively new headers; add them to the crypto component. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-4-peter.mayd...@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index fb081a59265..205ab23b280 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -79,7 +79,7 @@ chardev ~ .*/qemu((/include)?/chardev/.*) crypto - ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*) + ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*|/host/include/.*/host/crypto/.*) disas ~ .*/qemu((/include)?/disas.*) -- 2.34.1
[PULL 13/18] hw/arm/virt: Rename VIRT_UART and VIRT_SECURE_UART to VIRT_UART[01]
We're going to make the second UART not always a secure-only device.
Rename the constants VIRT_UART and VIRT_SECURE_UART to VIRT_UART0
and VIRT_UART1 accordingly.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240610162343.2131524-3-peter.mayd...@linaro.org
---
include/hw/arm/virt.h| 4 ++--
hw/arm/virt-acpi-build.c | 12 ++--
hw/arm/virt.c| 14 +++---
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index bb486d36b14..1227e7f7f08 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -59,7 +59,7 @@ enum {
VIRT_GIC_ITS,
VIRT_GIC_REDIST,
VIRT_SMMU,
-VIRT_UART,
+VIRT_UART0,
VIRT_MMIO,
VIRT_RTC,
VIRT_FW_CFG,
@@ -69,7 +69,7 @@ enum {
VIRT_PCIE_ECAM,
VIRT_PLATFORM_BUS,
VIRT_GPIO,
-VIRT_SECURE_UART,
+VIRT_UART1,
VIRT_SECURE_MEM,
VIRT_SECURE_GPIO,
VIRT_PCDIMM_ACPI,
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index c3ccfef026f..eb5796e309b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -440,10 +440,10 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
.base_addr.width = 32,
.base_addr.offset = 0,
.base_addr.size = 3,
-.base_addr.addr = vms->memmap[VIRT_UART].base,
+.base_addr.addr = vms->memmap[VIRT_UART0].base,
.interrupt_type = (1 << 3),/* Bit[3] ARMH GIC interrupt*/
.pc_interrupt = 0, /* IRQ */
-.interrupt = (vms->irqmap[VIRT_UART] + ARM_SPI_BASE),
+.interrupt = (vms->irqmap[VIRT_UART0] + ARM_SPI_BASE),
.baud_rate = 3,/* 9600 */
.parity = 0, /* No Parity */
.stop_bits = 1,/* 1 Stop bit */
@@ -631,11 +631,11 @@ build_dbg2(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
/* BaseAddressRegister[] */
build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 32, 0, 3,
- vms->memmap[VIRT_UART].base);
+ vms->memmap[VIRT_UART0].base);
/* AddressSize[] */
build_append_int_noprefix(table_data,
- vms->memmap[VIRT_UART].size, 4);
+ vms->memmap[VIRT_UART0].size, 4);
/* NamespaceString[] */
g_array_append_vals(table_data, name, namespace_length);
@@ -816,8 +816,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
*/
scope = aml_scope("\\_SB");
acpi_dsdt_add_cpus(scope, vms);
-acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
- (irqmap[VIRT_UART] + ARM_SPI_BASE));
+acpi_dsdt_add_uart(scope, &memmap[VIRT_UART0],
+ (irqmap[VIRT_UART0] + ARM_SPI_BASE));
if (vmc->acpi_expose_flash) {
acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 61a9d47c026..ffb4983885f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -165,11 +165,11 @@ static const MemMapEntry base_memmap[] = {
[VIRT_GIC_ITS] ={ 0x0808, 0x0002 },
/* This redistributor space allows up to 2*64kB*123 CPUs */
[VIRT_GIC_REDIST] = { 0x080A, 0x00F6 },
-[VIRT_UART] = { 0x0900, 0x1000 },
+[VIRT_UART0] = { 0x0900, 0x1000 },
[VIRT_RTC] ={ 0x0901, 0x1000 },
[VIRT_FW_CFG] = { 0x0902, 0x0018 },
[VIRT_GPIO] = { 0x0903, 0x1000 },
-[VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
+[VIRT_UART1] = { 0x0904, 0x1000 },
[VIRT_SMMU] = { 0x0905, 0x0002 },
[VIRT_PCDIMM_ACPI] ={ 0x0907, MEMORY_HOTPLUG_IO_LEN },
[VIRT_ACPI_GED] = { 0x0908, ACPI_GED_EVT_SEL_LEN },
@@ -212,11 +212,11 @@ static MemMapEntry extended_memmap[] = {
};
static const int a15irqmap[] = {
-[VIRT_UART] = 1,
+[VIRT_UART0] = 1,
[VIRT_RTC] = 2,
[VIRT_PCIE] = 3, /* ... to 6 */
[VIRT_GPIO] = 7,
-[VIRT_SECURE_UART] = 8,
+[VIRT_UART1] = 8,
[VIRT_ACPI_GED] = 9,
[VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
[VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
@@ -939,7 +939,7 @@ static void create_uart(const VirtMachineState *vms, int
uart,
qemu_fdt_setprop(ms->fdt, nodename, "clock-names",
clocknames, sizeof(clocknames));
-if (uart == VIRT_UART) {
+if (uart == VIRT_UART0) {
qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
qemu_fdt_setprop_string(ms->fdt, "/aliases", "serial0", nodename);
} else {
@@ -2317,11 +2317,11 @@ static void machvirt_init(MachineState *machine)
fdt_add_pmu_nodes(vms);
-create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
+create_uart(vms, VI
[PULL 08/18] scripts/coverity-scan/COMPONENTS.md: Fix monitor component
Update the 'monitor' component: * qapi/ and monitor/ are now subdirectories * add job-qmp.c Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-5-peter.mayd...@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 205ab23b280..3864f8eda07 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -97,7 +97,7 @@ migration ~ .*/qemu((/include)?/migration/.*) monitor - ~ .*/qemu(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*) + ~ .*/qemu((/include)?/(qapi|qobject|monitor)/.*|/job-qmp.c) nbd ~ .*/qemu(/nbd/.*|/include/block/nbd.*|/qemu-nbd\.c) -- 2.34.1
[PULL 00/18] target-arm queue
Hi; here's the latest target-arm pullreq; this is pretty much just various bugfixes. -- PMM The following changes since commit 02d9c38236cf8c9826e5c5be61780ccb4ae0: Merge tag 'pull-tcg-20240619' of https://gitlab.com/rth7680/qemu into staging (2024-06-19 14:00:39 -0700) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20240622 for you to fetch changes up to 3b36cead6ecc0e40edb8b2f3e253baa01ebc1e9a: hw/arm/sbsa-ref: Enable CPU cluster on ARM sbsa machine (2024-06-21 16:24:46 +0100) target-arm queue: * hw/net/can/xlnx-versal-canfd: Fix sorting of the tx queue * hw/arm/xilinx_zynq: Fix IRQ/FIQ routing * hw/intc/arm_gic: Fix deactivation of SPI lines * hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu * hw/misc: Set valid access size for Exynos4210 RNG * hw/arm/sbsa-ref: switch to 1GHz timer frequency * hw/arm/sbsa-ref: Enable CPU cluster on ARM sbsa machine * hw/arm/virt: allow creation of a second NonSecure UART * hw/arm/virt: Avoid unexpected warning from Linux guest on host with Fujitsu CPUs * scripts/coverity-scan/COMPONENTS.md: update component regexes * hw/usb/hcd-dwc2: Handle invalid address access in read and write functions * hw/usb/hcd-ohci: Fix ohci_service_td: accept zero-length TDs where CBP=BE+1 David Hubbard (1): hw/usb/hcd-ohci: Fix ohci_service_td: accept zero-length TDs where CBP=BE+1 Edgar E. Iglesias (1): hw/intc/arm_gic: Fix deactivation of SPI lines Marcin Juszkiewicz (1): hw/arm/sbsa-ref: switch to 1GHz timer frequency Peter Maydell (8): scripts/coverity-scan/COMPONENTS.md: Update paths to match gitlab CI scripts/coverity-scan/COMPONENTS.md: Fix 'char' component scripts/coverity-scan/COMPONENTS.md: Add crypto headers in host/include to the crypto component scripts/coverity-scan/COMPONENTS.md: Fix monitor component scripts/coverity-scan/COMPONENTS.md: Include libqmp in testlibs hw/arm/virt: Add serial aliases in DTB hw/arm/virt: Rename VIRT_UART and VIRT_SECURE_UART to VIRT_UART[01] hw/arm/virt: allow creation of a second NonSecure UART Sebastian Huber (1): hw/arm/xilinx_zynq: Fix IRQ/FIQ routing Shiva sagar Myana (1): hw/net/can/xlnx-versal-canfd: Fix sorting of the tx queue Xiong Yining (1): hw/arm/sbsa-ref: Enable CPU cluster on ARM sbsa machine Zhenyu Zhang (1): hw/arm/virt: Avoid unexpected warning from Linux guest on host with Fujitsu CPUs Zheyu Ma (3): hw/timer/a9gtimer: Handle QTest mode in a9_gtimer_get_current_cpu hw/usb/hcd-dwc2: Handle invalid address access in read and write functions hw/misc: Set valid access size for Exynos4210 RNG docs/system/arm/sbsa.rst| 4 ++ docs/system/arm/virt.rst| 6 +- hw/intc/gic_internal.h | 8 ++- include/hw/arm/virt.h | 5 +- hw/arm/sbsa-ref.c | 23 +--- hw/arm/virt-acpi-build.c| 22 +--- hw/arm/virt.c | 63 ++--- hw/arm/xilinx_zynq.c| 5 +- hw/misc/exynos4210_rng.c| 2 + hw/net/can/xlnx-versal-canfd.c | 5 +- hw/timer/a9gtimer.c | 5 ++ hw/usb/hcd-dwc2.c | 9 ++- hw/usb/hcd-ohci.c | 4 +- hw/usb/trace-events | 1 + scripts/coverity-scan/COMPONENTS.md | 107 ++-- 15 files changed, 179 insertions(+), 90 deletions(-)
[PULL 04/18] hw/arm/xilinx_zynq: Fix IRQ/FIQ routing
From: Sebastian Huber
Fix the system bus interrupt line to CPU core assignment.
Fixes: ddcf58e044ce0 ("hw/arm/xilinx_zynq: Support up to two CPU cores")
Signed-off-by: Sebastian Huber
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240610052906.4432-1-sebastian.hu...@embedded-brains.de
Signed-off-by: Peter Maydell
---
hw/arm/xilinx_zynq.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 7f7a3d23fbe..c79661bbc1b 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -252,10 +252,11 @@ static void zynq_init(MachineState *machine)
zynq_binfo.gic_cpu_if_addr = MPCORE_PERIPHBASE + 0x100;
sysbus_create_varargs("l2x0", MPCORE_PERIPHBASE + 0x2000, NULL);
for (n = 0; n < smp_cpus; n++) {
+/* See "hw/intc/arm_gic.h" for the IRQ line association */
DeviceState *cpudev = DEVICE(zynq_machine->cpu[n]);
-sysbus_connect_irq(busdev, (2 * n) + 0,
+sysbus_connect_irq(busdev, n,
qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
-sysbus_connect_irq(busdev, (2 * n) + 1,
+sysbus_connect_irq(busdev, smp_cpus + n,
qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
}
--
2.34.1
[PULL 06/18] scripts/coverity-scan/COMPONENTS.md: Fix 'char' component
The 'char' component: * includes the no-longer-present qemu-char.c, which has been long since split into the chardev/ backend code * also includes the hw/char devices Split it into two components: * char is the hw/char devices * chardev is the chardev backends with regexes matching our current sources. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240604145934.1230583-3-peter.mayd...@linaro.org --- scripts/coverity-scan/COMPONENTS.md | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md index 98d4bcd6a50..fb081a59265 100644 --- a/scripts/coverity-scan/COMPONENTS.md +++ b/scripts/coverity-scan/COMPONENTS.md @@ -73,7 +73,10 @@ block ~ .*/qemu(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*) char - ~ .*/qemu(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*) + ~ .*/qemu((/include)?/hw/char/.*) + +chardev + ~ .*/qemu((/include)?/chardev/.*) crypto ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*) -- 2.34.1
[PULL 17/18] hw/usb/hcd-ohci: Fix ohci_service_td: accept zero-length TDs where CBP=BE+1
From: David Hubbard
This changes the way the ohci emulation handles a Transfer Descriptor
with "Buffer End" set to "Current Buffer Pointer" - 1, specifically
in the case of a zero-length packet.
The OHCI spec 4.3.1.2 Table 4-2 specifies td.cbp to be zero for a
zero-length packet. Peter Maydell tracked down commit 1328fe0c32
(hw: usb: hcd-ohci: check len and frame_number variables) where qemu
started checking this according to the spec.
What this patch does is loosen the qemu ohci implementation to allow a
zero-length packet if td.be (Buffer End) is set to td.cbp - 1, and with a
non-zero td.cbp value.
The spec is unclear whether this is valid or not -- it is not the
clearly documented way to send a zero length TD (which is CBP=BE=0),
but it isn't specifically forbidden. Actual hw seems to be ok with it.
Does any OS rely on this behavior? There have been no reports to
qemu-devel of this problem.
This is attempting to have qemu behave like actual hardware,
but this is just a minor change.
With a tiny OS[1] that boots and executes a test, the issue can be seen:
* OS that sends USB requests to a USB mass storage device
but sends td.cbp = td.be + 1
* qemu 4.2
* qemu HEAD (4e66a0854)
* Actual OHCI controller (hardware)
Command line:
qemu-system-x86_64 -m 20 \
-device pci-ohci,id=ohci \
-drive if=none,format=raw,id=d,file=testmbr.raw \
-device usb-storage,bus=ohci.0,drive=d \
--trace "usb_*" --trace "ohci_*" -D qemu.log
Results are:
qemu 4.2 | qemu HEAD | actual HW
---++---
works fine | ohci_die() | works fine
Tip: if the flags "-serial pty -serial stdio" are added to the command line
the test will output USB requests like this:
Testing qemu HEAD:
> Free mem 2M ohci port2 conn FS
> setup { 80 6 0 1 0 0 8 0 }
> ED info=8 { mps=8 en=0 d=0 } tail=c20920
> td0 c20880 nxt=c20960 f200 setup cbp=c20900 be=c20907
> td1 c20960 nxt=c20980 f314in cbp=c20908 be=c2090f
> td2 c20980 nxt=c20920 f308 out cbp=c20910 be=c2090f ohci20 host err
> usb stopped
And in qemu.log:
usb_ohci_iso_td_bad_cc_overrun ISO_TD start_offset=0x00c20910 >
next_offset=0x00c2090f
Testing qemu 4.2:
> Free mem 2M ohci port2 conn FS
> setup { 80 6 0 1 0 0 8 0 }
> ED info=8 { mps=8 en=0 d=0 } tail=620920
> td0 620880 nxt=620960 f200 setup cbp=620900 be=620907 cbp=0
> be=620907
> td1 620960 nxt=620980 f314in cbp=620908 be=62090f cbp=0
> be=62090f
> td2 620980 nxt=620920 f308 out cbp=620910 be=62090f cbp=0
> be=62090f
>rx { 12 1 0 2 0 0 0 8 }
> setup { 0 5 1 0 0 0 0 0 } tx {}
> ED info=8 { mps=8 en=0 d=0 } tail=620880
> td0 620920 nxt=620960 f200 setup cbp=620900 be=620907 cbp=0
> be=620907
> td1 620960 nxt=620880 f310in cbp=620908 be=620907 cbp=0
> be=620907
> setup { 80 6 0 1 0 0 12 0 }
> ED info=80001 { mps=8 en=0 d=1 } tail=620960
> td0 620880 nxt=6209c0 f200 setup cbp=620920 be=620927 cbp=0
> be=620927
> td1 6209c0 nxt=6209e0 f314in cbp=620928 be=620939 cbp=0
> be=620939
> td2 6209e0 nxt=620960 f308 out cbp=62093a be=620939 cbp=0
> be=620939
>rx { 12 1 0 2 0 0 0 8 f4 46 1 0 0 0 1 2 3 1 }
> setup { 80 6 0 2 0 0 0 1 }
> ED info=80001 { mps=8 en=0 d=1 } tail=620880
> td0 620960 nxt=6209a0 f200 setup cbp=620a20 be=620a27 cbp=0
> be=620a27
> td1 6209a0 nxt=6209c0 f3140004in cbp=620a28 be=620b27 cbp=620a48
> be=620b27
> td2 6209c0 nxt=620880 f308 out cbp=620b28 be=620b27 cbp=0
> be=620b27
>rx { 9 2 20 0 1 1 4 c0 0 9 4 0 0 2 8 6 50 0 7 5 81 2 40 0 0 7 5 2 2 40 0 0
> }
> setup { 0 9 1 0 0 0 0 0 } tx {}
> ED info=80001 { mps=8 en=0 d=1 } tail=620900
> td0 620880 nxt=620940 f200 setup cbp=620a00 be=620a07 cbp=0
> be=620a07
> td1 620940 nxt=620900 f310in cbp=620a08 be=620a07 cbp=0
> be=620a07
[1] The OS disk image has been emailed to phi...@linaro.org, m...@tls.msk.ru,
and kra...@redhat.com:
* testCbpOffBy1.img.xz
* sha256: f87baddcb86de845de12f002c698670a426affb40946025cc32694f9daa3abed
Signed-off-by: David Hubbard
Reviewed-by: Alex Bennée
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/usb/hcd-ohci.c | 4 ++--
hw/usb/trace-events | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index acd60169802..71b54914d32 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -941,8 +941,8 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed
*ed)
if ((td.cbp & 0xf000) != (td.be & 0xf000)) {
len = (td.be & 0xfff) + 0x1001 - (td.cbp & 0xfff);
} else {
-if (td.cbp > td.be) {
-trace_usb_ohci_iso_td_bad_cc_overrun(td.cbp, td.be);
+if (td.cbp - 1 > td.be) { /* rely on td.cbp != 0 */
+trace_usb_ohci_td_bad_buf(td.cbp, td.be);
ohci_die(ohci);
return 1;
}
[PATCH] hw/usb/hcd-ohci: Set transfer error code with no dev
When a usb device is disconnected the transfer service functions bails
before appropraite transfer error flags are set.
This patch sets the appropriate condition code OHCI_CC_DEVICENOTRESPONDING
when a device is disconnected and consequently has no response on the USB bus.
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2081
Signed-off-by: Ryan Wendland
---
hw/usb/hcd-ohci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index acd6016980..8cd25d74af 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -980,7 +980,8 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed
*ed)
dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
if (dev == NULL) {
trace_usb_ohci_td_dev_error();
-return 1;
+OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DEVICENOTRESPONDING);
+goto exit_and_retire;
}
ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
if (ohci->async_td) {
@@ -1087,6 +1088,7 @@ static int ohci_service_td(OHCIState *ohci, struct
ohci_ed *ed)
ed->head |= OHCI_ED_H;
}
+exit_and_retire:
/* Retire this TD */
ed->head &= ~OHCI_DPTR_MASK;
ed->head |= td.next & OHCI_DPTR_MASK;
--
2.34.1
Re: [PULL 15/23] Revert "host/i386: assume presence of SSE2"
On 6/21/24 23:15, Paolo Bonzini wrote:
This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
util/bufferiszero.c | 4 ++--
util/cpuinfo-i386.c | 1 +
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index 72f6fad61e5..81771733eaa 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -14,6 +14,7 @@
#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
+#define CPUINFO_SSE2(1u << 7)
#define CPUINFO_AVX1(1u << 9)
#define CPUINFO_AVX2(1u << 10)
#define CPUINFO_AVX512F (1u << 11)
diff --git a/util/bufferiszero.c b/util/bufferiszero.c
index 11c080e02cf..74864f7b782 100644
--- a/util/bufferiszero.c
+++ b/util/bufferiszero.c
@@ -188,14 +188,14 @@ static biz_accel_fn const accel_table[] = {
static unsigned best_accel(void)
{
-#ifdef CONFIG_AVX2_OPT
unsigned info = cpuinfo_init();
+#ifdef CONFIG_AVX2_OPT
if (info & CPUINFO_AVX2) {
return 2;
}
#endif
-return 1;
+return info & CPUINFO_SSE2 ? 1 : 0;
}
Merge conflict with master here -- bufferiszero.c has been split.
This hunk now goes in host/include/i386/host/bufferiszero.c.inc.
r~
Re: [PATCH 01/23] Add CPU initialization function
On Mon, Jun 17, 2024 at 10:17 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 6/17/24 11:57, Ajeet Singh wrote:
> > From: Stacey Son
> >
> > Addded function to initialize ARM CPU
> > and to check if it supports 64 bit mode
> >
> > Signed-off-by: Ajeet Singh
> > Signed-off-by: Stacey Son
> > ---
> > bsd-user/aarch64/target_arch_cpu.h | 42 ++
> > 1 file changed, 42 insertions(+)
> > create mode 100644 bsd-user/aarch64/target_arch_cpu.h
> >
> > diff --git a/bsd-user/aarch64/target_arch_cpu.h
> b/bsd-user/aarch64/target_arch_cpu.h
> > new file mode 100644
> > index 00..db5c7062b9
> > --- /dev/null
> > +++ b/bsd-user/aarch64/target_arch_cpu.h
> > @@ -0,0 +1,42 @@
> > +/*
> > + * ARM AArch64 cpu init and loop
> > + *
> > + * Copyright (c) 2015 Stacey Son
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef TARGET_ARCH_CPU_H
> > +#define TARGET_ARCH_CPU_H
> > +
> > +#include "target_arch.h"
> > +#include "target/arm/syndrome.h"
>
> Do you actually need syndrome.h?
>
It's needed, but not for this chunk. It is needed for patch 2 because we
start to use the syndrome functions there to dispatch / decode the traps.
So that should be moved to patch 2 in the next round, I think.
Also
Reviewed-by: Warner Losh
since this looks correct and I didn't write it :)
Warner
> Otherwise,
> Reviewed-by: Richard Henderson
>
> r~
>
> > +
> > +#define TARGET_DEFAULT_CPU_MODEL "any"
> > +
> > +static inline void target_cpu_init(CPUARMState *env,
> > +struct target_pt_regs *regs)
> > +{
> > +int i;
> > +
> > +if (!(arm_feature(env, ARM_FEATURE_AARCH64))) {
> > +fprintf(stderr, "The selected ARM CPU does not support 64 bit
> mode\n");
> > +exit(1);
> > +}
> > +for (i = 0; i < 31; i++) {
> > +env->xregs[i] = regs->regs[i];
> > +}
> > +env->pc = regs->pc;
> > +env->xregs[31] = regs->sp;
> > +}
>
>
Re: [PATCH 02/23] Added CPU loop function
On Mon, Jun 17, 2024 at 10:24 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 6/17/24 11:57, Ajeet Singh wrote:
> > +/*
> > + * The carry bit is cleared for no error; set for error.
> > + * See arm64/arm64/vm_machdep.c cpu_set_syscall_retval()
> > + */
> > +pstate = pstate_read(env);
> > +if (ret >= 0) {
> > +pstate &= ~PSTATE_C;
> > +env->xregs[0] = ret;
> > +} else if (ret == -TARGET_ERESTART) {
> > +env->pc -= 4;
> > +break;
> > +} else if (ret != -TARGET_EJUSTRETURN) {
> > +pstate |= PSTATE_C;
> > +env->xregs[0] = -ret;
> > +}
> > +pstate_write(env, pstate);
>
> No need for full pstate read/write:
>
> env->CF = {0,1};
>
If I understand what you're suggesting, the quoted code can be replaced
by the following, faster construct:
/*
* The carry bit is cleared for no error; set for error.
* See arm64/arm64/vm_machdep.c cpu_set_syscall_retval()
*/
if (ret >= 0) {
env->CF = 0;
env->xregs[0] = ret;
} else if (ret == -TARGET_ERESTART) {
env->pc -= 4;
break;
} else if (ret != -TARGET_EJUSTRETURN) {
env->CF = 1;
env->xregs[0] = -ret;
}
break;
Is that what you're saying?
> > +break;
> > +
> > +case EXCP_INTERRUPT:
> > +/* Just indicate that signals should be handle ASAP. */
> > +break;
> > +
> > +case EXCP_UDEF:
> > +force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
> > +break;
> > +
> > +
> > +case EXCP_PREFETCH_ABORT:
> > +case EXCP_DATA_ABORT:
> > +/* We should only arrive here with EC in {DATAABORT,
> INSNABORT}. */
> > +ec = syn_get_ec(env->exception.syndrome);
>
> Nevermind about my question about syndrome.h vs patch 1.
>
Ah, Since we have to re-roll this patch anyway, maybe moving it is a good
idea?
Honestly, I'm good either way.
Warner
> r~
>
Re: [PATCH 11/23] Update ARM AArch64 VM parameter definitions for bsd-user
On Tue, Jun 18, 2024 at 4:16 PM Richard Henderson < richard.hender...@linaro.org> wrote: > On 6/17/24 11:57, Ajeet Singh wrote: > > From: Stacey Son > > > > Defined address spaces for FreeBSD/arm64 and added function for > > getting stack pointer from CPU and setting a return value. > > > > Signed-off-by: Stacey Son > > Signed-off-by: Warner Losh > > Signed-off-by: Ajeet Singh > > Co-authored-by: Sean Bruno > > Co-authored-by: Warner Losh > > --- > > bsd-user/aarch64/target_arch_vmparam.h | 68 ++ > > 1 file changed, 68 insertions(+) > > create mode 100644 bsd-user/aarch64/target_arch_vmparam.h > > Acked-by: Richard Henderson > > > +/* KERNBASE - 512 MB */ > > +#define TARGET_VM_MAXUSER_ADDRESS (0x7f00ULL - (512 * > MiB)) > > +#define TARGET_USRSTACK TARGET_VM_MAXUSER_ADDRESS > > I will note that this may conflict with -R reserved_size, > and is an existing issue with the x86_64 port as well. > There are indeed existing issues with address space management. We're working through them right now in the blitz branch. We have finally found where the atomic issues were coming from and it is not setting the flag saying we want atomic ops when creating the CPU structures (that's a quick summary, I'll post more on this later when we review it). So I'd suggest, for the moment, allowing this in and fixing it when we get those details ironed out. Does that sound OK? Warner
[PATCH] target/ppc/mem_helper.c: Remove a conditional from dcbz_common()
Instead of passing a bool and select a value within dcbz_common() let
the callers pass in the right value to avoid this conditional
statement. On PPC dcbz is often used to zero memory and some code uses
it a lot. This change improves the run time of a test case that copies
memory with a dcbz call in every iteration from 6.23 to 5.83 seconds.
Signed-off-by: BALATON Zoltan
---
This is just a small optimisation removing some of the overhead but
dcbz still seems to be the biggest issue with this test. Removing the
dcbz call it runs in 2 seconds. In a profile I see:
Children Self Command Shared ObjectSymbol
- 55.01%11.44% qemu-ppc qemu-ppc [.]
dcbz_common.constprop.0
- 43.57% dcbz_common.constprop.0
- probe_access
- page_get_flags
interval_tree_iter_first
- 11.44% helper_raise_exception_err
cpu_loop_exit_restore
cpu_loop
cpu_exec
cpu_exec_setjmp.isra.0
cpu_exec_loop.constprop.0
cpu_tb_exec
0x7f262403636e
helper_raise_exception_err
cpu_loop_exit_restore
cpu_loop
cpu_exec
cpu_exec_setjmp.isra.0
cpu_exec_loop.constprop.0
cpu_tb_exec
- 0x7f26240386a4
11.20% helper_dcbz
+ 43.81%12.28% qemu-ppc qemu-ppc [.] probe_access
+ 39.31% 0.00% qemu-ppc [JIT] tid 9969 [.] 0x7f262400
+ 32.45% 4.51% qemu-ppc qemu-ppc [.] page_get_flags
+ 25.50% 2.10% qemu-ppc qemu-ppc [.]
interval_tree_iter_first
+ 24.67%24.67% qemu-ppc qemu-ppc [.]
interval_tree_subtree_search
+ 16.75% 1.19% qemu-ppc qemu-ppc [.] helper_dcbz
+4.78% 4.78% qemu-ppc [JIT] tid 9969 [.] 0x7f26240386be
+3.46% 3.46% qemu-ppc libc-2.32.so [.]
__memset_avx2_unaligned_erms
Any idea how this could be optimised further? (This is running with
qemu-ppc user mode emulation but I think with system it might be even
worse.) Could an inline implementation with TCG vector ops work to
avoid the helper and let it compile to efficient host code? Even if
that could work I don't know how to do that so I'd need some further
advice on this.
target/ppc/mem_helper.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index f88155ad45..361fd72226 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -271,12 +271,11 @@ void helper_stsw(CPUPPCState *env, target_ulong addr,
uint32_t nb,
}
static void dcbz_common(CPUPPCState *env, target_ulong addr,
-uint32_t opcode, bool epid, uintptr_t retaddr)
+uint32_t opcode, int mmu_idx, uintptr_t retaddr)
{
target_ulong mask, dcbz_size = env->dcache_line_size;
uint32_t i;
void *haddr;
-int mmu_idx = epid ? PPC_TLB_EPID_STORE : ppc_env_mmu_index(env, false);
#if defined(TARGET_PPC64)
/* Check for dcbz vs dcbzl on 970 */
@@ -309,12 +308,12 @@ static void dcbz_common(CPUPPCState *env, target_ulong
addr,
void helper_dcbz(CPUPPCState *env, target_ulong addr, uint32_t opcode)
{
-dcbz_common(env, addr, opcode, false, GETPC());
+dcbz_common(env, addr, opcode, ppc_env_mmu_index(env, false), GETPC());
}
void helper_dcbzep(CPUPPCState *env, target_ulong addr, uint32_t opcode)
{
-dcbz_common(env, addr, opcode, true, GETPC());
+dcbz_common(env, addr, opcode, PPC_TLB_EPID_STORE, GETPC());
}
void helper_icbi(CPUPPCState *env, target_ulong addr)
--
2.30.9
[PATCH v15 03/14] virtio-gpu: Move print_stats timer to VirtIOGPUGL
Move print_stats timer to VirtIOGPUGL for consistency with
cmdq_resume_bh and fence_poll that are used only by GL device.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 10 ++
include/hw/virtio/virtio-gpu.h | 2 +-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 91dce90f9176..a63d1f540f04 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -574,6 +574,7 @@ static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
static void virtio_gpu_print_stats(void *opaque)
{
VirtIOGPU *g = opaque;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
if (g->stats.requests) {
fprintf(stderr, "stats: vq req %4d, %3d -- 3D %4d (%5d)\n",
@@ -588,7 +589,7 @@ static void virtio_gpu_print_stats(void *opaque)
} else {
fprintf(stderr, "stats: idle\r");
}
-timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
+timer_mod(gl->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
static void virtio_gpu_fence_poll(void *opaque)
@@ -651,9 +652,10 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
virtio_gpu_fence_poll, g);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
- virtio_gpu_print_stats, g);
-timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
1000);
+gl->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+ virtio_gpu_print_stats, g);
+timer_mod(gl->print_stats,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
return 0;
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index bc69fd78a440..7ff989a45a5c 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -196,7 +196,6 @@ struct VirtIOGPU {
uint64_t hostmem;
bool processing_cmdq;
-QEMUTimer *print_stats;
uint32_t inflight;
struct {
@@ -232,6 +231,7 @@ struct VirtIOGPUGL {
bool renderer_reset;
QEMUTimer *fence_poll;
+QEMUTimer *print_stats;
};
struct VhostUserGPU {
--
2.45.2
[PATCH v15 08/14] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled
The udmabuf usage is mandatory when virgl is disabled and blobs feature
enabled in the Qemu machine configuration. If virgl and blobs are enabled,
then udmabuf requirement is optional. Since udmabuf isn't widely supported
by a popular Linux distros today, let's relax the udmabuf requirement for
blobs=on,virgl=on. Now, a full-featured virtio-gpu acceleration is
available to Qemu users without a need to have udmabuf available in the
system.
Reviewed-by: Antonio Caggiano
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Reviewed-by: Marc-André Lureau
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 602952a7041b..40a9d089710c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1485,6 +1485,7 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error
**errp)
if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
+!virtio_gpu_virgl_enabled(g->parent_obj.conf) &&
!virtio_gpu_have_udmabuf()) {
error_setg(errp, "need rutabaga or udmabuf for blob resources");
return;
--
2.45.2
[PATCH v15 12/14] virtio-gpu: Handle resource blob commands
From: Antonio Caggiano
Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true
Signed-off-by: Antonio Caggiano
Signed-off-by: Xenia Ragiadakou
Signed-off-by: Huang Rui
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 3 +
hw/display/virtio-gpu-virgl.c | 334 +++--
hw/display/virtio-gpu.c| 6 +-
include/hw/virtio/virtio-gpu.h | 2 +
4 files changed, 330 insertions(+), 15 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 4fe9e6a0c21c..5f27568d3ec8 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -160,6 +160,9 @@ static void virtio_gpu_gl_device_unrealize(DeviceState
*qdev)
VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
if (gl->renderer_state >= RS_INITED) {
+#if VIRGL_VERSION_MAJOR >= 1
+qemu_bh_delete(gl->cmdq_resume_bh);
+#endif
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
timer_free(gl->print_stats);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 60befab7efc2..f6cb4fe5b28e 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -26,6 +26,7 @@
struct virtio_gpu_virgl_resource {
struct virtio_gpu_simple_resource base;
+MemoryRegion *mr;
};
static struct virtio_gpu_virgl_resource *
@@ -49,6 +50,152 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
}
#endif
+#if VIRGL_VERSION_MAJOR >= 1
+typedef enum {
+HOSTMEM_MR_UNMAPPING,
+HOSTMEM_MR_FINISH_UNMAPPING,
+} HostmemMRState;
+
+struct virtio_gpu_virgl_hostmem_region {
+MemoryRegion mr;
+struct VirtIOGPU *g;
+HostmemMRState state;
+};
+
+static struct virtio_gpu_virgl_hostmem_region *
+to_hostmem_region(MemoryRegion *mr)
+{
+return container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
+}
+
+static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
+{
+VirtIOGPU *g = opaque;
+
+virtio_gpu_process_cmdq(g);
+}
+
+static void virtio_gpu_virgl_hostmem_region_free(void *obj)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b;
+VirtIOGPUGL *gl;
+
+vmr = to_hostmem_region(mr);
+vmr->state = HOSTMEM_MR_FINISH_UNMAPPING;
+
+b = VIRTIO_GPU_BASE(vmr->g);
+b->renderer_blocked--;
+
+/*
+ * memory_region_unref() is executed from RCU thread context, while
+ * virglrenderer works only on the main-loop thread that's holding GL
+ * context.
+ */
+gl = VIRTIO_GPU_GL(vmr->g);
+qemu_bh_schedule(gl->cmdq_resume_bh);
+}
+
+static int
+virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource *res,
+ uint64_t offset)
+{
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr;
+uint64_t size;
+void *data;
+int ret;
+
+if (!virtio_gpu_hostmem_enabled(b->conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__);
+return -EOPNOTSUPP;
+}
+
+ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource:
%s\n",
+ __func__, strerror(-ret));
+return ret;
+}
+
+vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
+vmr->g = g;
+
+mr = &vmr->mr;
+memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
+memory_region_add_subregion(&b->hostmem, offset, mr);
+memory_region_set_enabled(mr, true);
+
+/*
+ * MR could outlive the resource if MR's reference is held outside of
+ * virtio-gpu. In order to prevent unmapping resource while MR is alive,
+ * and thus, making the data pointer invalid, we will block virtio-gpu
+ * command processing until MR is fully unreferenced and freed.
+ */
+OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+res->mr = mr;
+
+return 0;
+}
+
+static int
+virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource *res,
+ bool *cmd_suspended)
+{
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr = res->mr;
+int ret;
+
+if (!mr) {
+return 0;
+}
+
+vmr = to_hostmem_region(res->mr);
+
+/*
+ * Perform async unmapping in 3 steps:
+ *
+ * 1. Begin async unmapping with memory_region_del_subregion()
+ *and suspend/block cmd processing.
+ * 2. Wait for res->mr to be freed and cmd processing resumed
+ *asynchronously by virtio_gpu_virgl_hostmem_region_free().
+ * 3. Finish the unmappi
[PATCH v15 14/14] virtio-gpu: Support Venus context
From: Antonio Caggiano
Request Venus when initializing VirGL and if venus=true flag is set for
virtio-gpu-gl device.
Signed-off-by: Antonio Caggiano
Signed-off-by: Huang Rui
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 2 ++
hw/display/virtio-gpu-virgl.c | 22 ++
hw/display/virtio-gpu.c| 15 +++
include/hw/virtio/virtio-gpu.h | 3 +++
4 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 20a7c316bb23..9be452547322 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -151,6 +151,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev,
Error **errp)
static Property virtio_gpu_gl_properties[] = {
DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
VIRTIO_GPU_FLAG_STATS_ENABLED, false),
+DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 58693dfa2afa..08b0e7e49337 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1135,6 +1135,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE;
}
#endif
+#if VIRGL_VERSION_MAJOR >= 1
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
+}
+#endif
ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
if (ret != 0) {
@@ -1168,7 +1173,7 @@ static void virtio_gpu_virgl_add_capset(GArray
*capset_ids, uint32_t capset_id)
GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
{
-uint32_t capset2_max_ver, capset2_max_size;
+uint32_t capset_max_ver, capset_max_size;
GArray *capset_ids;
capset_ids = g_array_new(false, false, sizeof(uint32_t));
@@ -1177,11 +1182,20 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
- &capset2_max_ver,
- &capset2_max_size);
-if (capset2_max_ver) {
+ &capset_max_ver,
+ &capset_max_size);
+if (capset_max_ver) {
virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
}
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+ &capset_max_ver,
+ &capset_max_size);
+if (capset_max_size) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VENUS);
+}
+}
+
return capset_ids;
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index a5db2256a4bb..50b5634af13f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1507,6 +1507,21 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error
**errp)
#endif
}
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+#ifdef VIRGL_VERSION_MAJOR
+#if VIRGL_VERSION_MAJOR >= 1
+if (!virtio_gpu_blob_enabled(g->parent_obj.conf) ||
+!virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
+error_setg(errp, "venus requires enabled blob and hostmem
options");
+return;
+}
+#else
+error_setg(errp, "old virglrenderer, venus unsupported");
+return;
+#endif
+#endif
+}
+
if (!virtio_gpu_base_device_realize(qdev,
virtio_gpu_handle_ctrl_cb,
virtio_gpu_handle_cursor_cb,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 83232f4b4bfa..230fa0c4ee0a 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -99,6 +99,7 @@ enum virtio_gpu_base_conf_flags {
VIRTIO_GPU_FLAG_BLOB_ENABLED,
VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
+VIRTIO_GPU_FLAG_VENUS_ENABLED,
};
#define virtio_gpu_virgl_enabled(_cfg) \
@@ -117,6 +118,8 @@ enum virtio_gpu_base_conf_flags {
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
#define virtio_gpu_hostmem_enabled(_cfg) \
(_cfg.hostmem > 0)
+#define virtio_gpu_venus_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED))
struct virtio_gpu_base_conf {
uint32_t max_outputs;
--
2.45.2
[PATCH v15 05/14] virtio-gpu: Unrealize GL device
Even though GL GPU doesn't support hotplugging today, free virgl
resources when GL device is unrealized. For consistency.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 21a1e9a05c5d..0109244276fc 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -150,6 +150,22 @@ static Property virtio_gpu_gl_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
+{
+VirtIOGPU *g = VIRTIO_GPU(qdev);
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
+
+if (gl->renderer_state >= RS_INITED) {
+if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+timer_free(gl->print_stats);
+}
+timer_free(gl->fence_poll);
+virgl_renderer_cleanup(NULL);
+}
+
+gl->renderer_state = RS_START;
+}
+
static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -163,6 +179,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass,
void *data)
vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
vdc->realize = virtio_gpu_gl_device_realize;
+vdc->unrealize = virtio_gpu_gl_device_unrealize;
vdc->reset = virtio_gpu_gl_reset;
device_class_set_props(dc, virtio_gpu_gl_properties);
}
--
2.45.2
[PATCH v15 11/14] virtio-gpu: Support suspension of commands processing
Check whether command processing has been finished; otherwise, stop
processing commands and retry the command again next time. This allows
us to support asynchronous execution of non-fenced commands needed for
unmapping host blobs safely.
Suggested-by: Akihiko Odaki
Signed-off-by: Dmitry Osipenko
---
hw/display/trace-events | 1 +
hw/display/virtio-gpu.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index e212710284ae..d26d663f9638 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -55,6 +55,7 @@ virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence
0x%" PRIx64 ", type
virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64
virtio_gpu_inc_inflight_fences(uint32_t inflight) "in-flight+ %u"
virtio_gpu_dec_inflight_fences(uint32_t inflight) "in-flight- %u"
+virtio_gpu_cmd_suspended(uint32_t cmd) "cmd 0x%x"
# qxl.c
disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t
val) "%d %s addr=%u val=%u"
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 95091c4b7924..1c6e97fb6931 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1054,6 +1054,12 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
/* process command */
vgc->process_cmd(g, cmd);
+/* command suspended */
+if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
+trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type);
+break;
+}
+
QTAILQ_REMOVE(&g->cmdq, cmd, next);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
g->stats.requests++;
--
2.45.2
[PATCH v15 04/14] virtio-gpu: Handle virtio_gpu_virgl_init() failure
virtio_gpu_virgl_init() may fail, leading to a further Qemu crash
because Qemu assumes it never fails. Check virtio_gpu_virgl_init()
return code and don't execute virtio commands on error. Failed
virtio_gpu_virgl_init() will result in a timed out virtio commands
for a guest OS.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 30 ++
include/hw/virtio/virtio-gpu.h | 11 +--
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e06be60dfbfc..21a1e9a05c5d 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -29,9 +29,14 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
struct virtio_gpu_scanout *s,
uint32_t resource_id)
{
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
uint32_t width, height;
uint32_t pixels, *data;
+if (gl->renderer_state != RS_INITED) {
+return;
+}
+
data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
if (!data) {
return;
@@ -65,13 +70,22 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
return;
}
-if (!gl->renderer_inited) {
-virtio_gpu_virgl_init(g);
-gl->renderer_inited = true;
-}
-if (gl->renderer_reset) {
-gl->renderer_reset = false;
+switch (gl->renderer_state) {
+case RS_RESET:
virtio_gpu_virgl_reset(g);
+/* fallthrough */
+case RS_START:
+if (virtio_gpu_virgl_init(g)) {
+gl->renderer_state = RS_INIT_FAILED;
+return;
+}
+
+gl->renderer_state = RS_INITED;
+break;
+case RS_INIT_FAILED:
+return;
+case RS_INITED:
+break;
}
cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
@@ -98,9 +112,9 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
* GL functions must be called with the associated GL context in main
* thread, and when the renderer is unblocked.
*/
-if (gl->renderer_inited && !gl->renderer_reset) {
+if (gl->renderer_state == RS_INITED) {
virtio_gpu_virgl_reset_scanout(g);
-gl->renderer_reset = true;
+gl->renderer_state = RS_RESET;
}
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7ff989a45a5c..6e71d799e5da 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -224,11 +224,18 @@ struct VirtIOGPUClass {
Error **errp);
};
+/* VirtIOGPUGL renderer states */
+typedef enum {
+RS_START, /* starting state */
+RS_INIT_FAILED, /* failed initialisation */
+RS_INITED, /* initialised and working */
+RS_RESET, /* inited and reset pending, moves to start after reset */
+} RenderState;
+
struct VirtIOGPUGL {
struct VirtIOGPU parent_obj;
-bool renderer_inited;
-bool renderer_reset;
+RenderState renderer_state;
QEMUTimer *fence_poll;
QEMUTimer *print_stats;
--
2.45.2
[PATCH v15 02/14] virtio-gpu: Move fence_poll timer to VirtIOGPUGL
Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh
that are used only by GL device.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 8 +---
include/hw/virtio/virtio-gpu.h | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 14091b191ec0..91dce90f9176 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -594,11 +594,12 @@ static void virtio_gpu_print_stats(void *opaque)
static void virtio_gpu_fence_poll(void *opaque)
{
VirtIOGPU *g = opaque;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
virgl_renderer_poll();
virtio_gpu_process_cmdq(g);
if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
-timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
+timer_mod(gl->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
}
}
@@ -626,6 +627,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
{
int ret;
uint32_t flags = 0;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
if (qemu_egl_display) {
@@ -645,8 +647,8 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
return ret;
}
-g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL,
- virtio_gpu_fence_poll, g);
+gl->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+ virtio_gpu_fence_poll, g);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7a59379f5a7a..bc69fd78a440 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -196,7 +196,6 @@ struct VirtIOGPU {
uint64_t hostmem;
bool processing_cmdq;
-QEMUTimer *fence_poll;
QEMUTimer *print_stats;
uint32_t inflight;
@@ -231,6 +230,8 @@ struct VirtIOGPUGL {
bool renderer_inited;
bool renderer_reset;
+
+QEMUTimer *fence_poll;
};
struct VhostUserGPU {
--
2.45.2
[PATCH v15 09/14] virtio-gpu: Add virgl resource management
From: Huang Rui
In a preparation to adding host blobs support to virtio-gpu, add virgl
resource management that allows to retrieve resource based on its ID
and virgl resource wrapper on top of simple resource that will be contain
fields specific to virgl.
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 76 +++
1 file changed, 76 insertions(+)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b3aa444bcfa5..3ffea478e723 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -22,6 +22,23 @@
#include
+struct virtio_gpu_virgl_resource {
+struct virtio_gpu_simple_resource base;
+};
+
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
+{
+struct virtio_gpu_simple_resource *res;
+
+res = virtio_gpu_find_resource(g, resource_id);
+if (!res) {
+return NULL;
+}
+
+return container_of(res, struct virtio_gpu_virgl_resource, base);
+}
+
#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
static void *
virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
@@ -35,11 +52,34 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
{
struct virtio_gpu_resource_create_2d c2d;
struct virgl_renderer_resource_create_args args;
+struct virtio_gpu_virgl_resource *res;
VIRTIO_GPU_FILL_CMD(c2d);
trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
c2d.width, c2d.height);
+if (c2d.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, c2d.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, c2d.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_virgl_resource, 1);
+res->base.width = c2d.width;
+res->base.height = c2d.height;
+res->base.format = c2d.format;
+res->base.resource_id = c2d.resource_id;
+QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
args.handle = c2d.resource_id;
args.target = 2;
args.format = c2d.format;
@@ -59,11 +99,34 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
{
struct virtio_gpu_resource_create_3d c3d;
struct virgl_renderer_resource_create_args args;
+struct virtio_gpu_virgl_resource *res;
VIRTIO_GPU_FILL_CMD(c3d);
trace_virtio_gpu_cmd_res_create_3d(c3d.resource_id, c3d.format,
c3d.width, c3d.height, c3d.depth);
+if (c3d.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, c3d.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, c3d.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_virgl_resource, 1);
+res->base.width = c3d.width;
+res->base.height = c3d.height;
+res->base.format = c3d.format;
+res->base.resource_id = c3d.resource_id;
+QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
args.handle = c3d.resource_id;
args.target = c3d.target;
args.format = c3d.format;
@@ -82,12 +145,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
struct virtio_gpu_resource_unref unref;
+struct virtio_gpu_virgl_resource *res;
struct iovec *res_iovs = NULL;
int num_iovs = 0;
VIRTIO_GPU_FILL_CMD(unref);
trace_virtio_gpu_cmd_res_unref(unref.resource_id);
+res = virtio_gpu_virgl_find_resource(g, unref.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, unref.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
@@ -95,6 +167,10 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
}
virgl_renderer_resource_unref(unref.resource_id);
+
+QTAILQ_REMOVE(&g->reslist, &res->base, next);
+
+g_free(res);
}
static void virgl_cmd_context_create(VirtIOGPU *g,
--
2.45.2
[PATCH v15 06/14] virtio-gpu: Use pkgconfig version to decide which virgl features are available
New virglrerenderer features were stabilized with release of v1.0.0.
Presence of symbols in virglrenderer.h doesn't guarantee ABI compatibility
with pre-release development versions of libvirglerender. Use virglrenderer
version to decide reliably which virgl features are available.
Reviewed-by: Alex Bennée
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 2 +-
meson.build | 5 +
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index a63d1f540f04..ca6f4d6cbb58 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -171,7 +171,7 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g,
struct virgl_renderer_resource_info info;
void *d3d_tex2d = NULL;
-#ifdef HAVE_VIRGL_D3D_INFO_EXT
+#if VIRGL_VERSION_MAJOR >= 1
struct virgl_renderer_resource_info_ext ext;
memset(&ext, 0, sizeof(ext));
ret = virgl_renderer_resource_get_info_ext(ss.resource_id, &ext);
diff --git a/meson.build b/meson.build
index 97e00d6f59b8..838d08ef0f9b 100644
--- a/meson.build
+++ b/meson.build
@@ -2329,10 +2329,7 @@ config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
if virgl.found()
- config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT',
- cc.has_member('struct
virgl_renderer_resource_info_ext', 'd3d_tex2d',
- prefix: '#include ',
- dependencies: virgl))
+ config_host_data.set('VIRGL_VERSION_MAJOR', virgl.version().split('.')[0])
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.45.2
[PATCH v15 07/14] virtio-gpu: Support context-init feature with virglrenderer
From: Huang Rui
Patch "virtio-gpu: CONTEXT_INIT feature" has added the context_init
feature flags. Expose this feature and support creating virglrenderer
context with flags using context_id if libvirglrenderer is new enough.
Originally-by: Antonio Caggiano
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c| 4
hw/display/virtio-gpu-virgl.c | 20 ++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 0109244276fc..4fe9e6a0c21c 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -141,6 +141,10 @@ static void virtio_gpu_gl_device_realize(DeviceState
*qdev, Error **errp)
VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
virtio_gpu_virgl_get_num_capsets(g);
+#if VIRGL_VERSION_MAJOR >= 1
+g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
+#endif
+
virtio_gpu_device_realize(qdev, errp);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index ca6f4d6cbb58..b3aa444bcfa5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -106,8 +106,24 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
cc.debug_name);
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
- cc.debug_name);
+if (cc.context_init) {
+if (!virtio_gpu_context_init_enabled(g->parent_obj.conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: context_init disabled",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+
+#if VIRGL_VERSION_MAJOR >= 1
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
}
static void virgl_cmd_context_destroy(VirtIOGPU *g,
--
2.45.2
[PATCH v15 10/14] virtio-gpu: Support blob scanout using dmabuf fd
From: Robert Beckett
Support displaying blob resources by handling SET_SCANOUT_BLOB
command.
Signed-by: Antonio Caggiano
Signed-off-by: Robert Beckett
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 109 +
hw/display/virtio-gpu.c| 12 ++--
include/hw/virtio/virtio-gpu.h | 7 +++
3 files changed, 122 insertions(+), 6 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 3ffea478e723..60befab7efc2 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -17,6 +17,8 @@
#include "trace.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-gpu-pixman.h"
#include "ui/egl-helpers.h"
@@ -78,6 +80,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
res->base.height = c2d.height;
res->base.format = c2d.format;
res->base.resource_id = c2d.resource_id;
+res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c2d.resource_id;
@@ -125,6 +128,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
res->base.height = c3d.height;
res->base.format = c3d.format;
res->base.resource_id = c3d.resource_id;
+res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c3d.resource_id;
@@ -509,6 +513,106 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
g_free(resp);
}
+#if VIRGL_VERSION_MAJOR >= 1
+static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_framebuffer fb = { 0 };
+struct virgl_renderer_resource_info info;
+struct virtio_gpu_virgl_resource *res;
+struct virtio_gpu_set_scanout_blob ss;
+uint64_t fbend;
+
+VIRTIO_GPU_FILL_CMD(ss);
+virtio_gpu_scanout_blob_bswap(&ss);
+trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
+ ss.r.width, ss.r.height, ss.r.x,
+ ss.r.y);
+
+if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
+ __func__, ss.scanout_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+return;
+}
+
+if (ss.resource_id == 0) {
+virtio_gpu_disable_scanout(g, ss.scanout_id);
+return;
+}
+
+if (ss.width < 16 ||
+ss.height < 16 ||
+ss.r.x + ss.r.width > ss.width ||
+ss.r.y + ss.r.height > ss.height) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
+ " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
+ __func__, ss.scanout_id, ss.resource_id,
+ ss.r.x, ss.r.y, ss.r.width, ss.r.height,
+ ss.width, ss.height);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, ss.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (virgl_renderer_resource_get_info(ss.resource_id, &info)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not have info %d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (res->base.dmabuf_fd < 0) {
+res->base.dmabuf_fd = info.fd;
+}
+if (res->base.dmabuf_fd < 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource not backed by dmabuf
%d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+fb.format = virtio_gpu_get_pixman_format(ss.format);
+if (!fb.format) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n",
+ __func__, ss.format);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+fb.width = ss.width;
+fb.height = ss.height;
+fb.stride = ss.strides[0];
+fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+
+fbend = fb.offset;
+fbend += fb.stride * (ss.r.height - 1);
+fbend += fb.bytes_pp * ss.r.width;
+if (fbend > res->base.blob_size) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+g->parent_obj.enable = 1;
+if (virtio
[PATCH v15 13/14] virtio-gpu: Register capsets dynamically
From: Pierre-Eric Pelloux-Prayer
virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
like Venus and DRM capsets. Register capsets dynamically to avoid that problem.
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Pierre-Eric Pelloux-Prayer
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 6 --
hw/display/virtio-gpu-virgl.c | 33 +
include/hw/virtio/virtio-gpu.h | 4 +++-
3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 5f27568d3ec8..20a7c316bb23 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -138,8 +138,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev,
Error **errp)
}
g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
-VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
-virtio_gpu_virgl_get_num_capsets(g);
+g->capset_ids = virtio_gpu_virgl_get_capsets(g);
+VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len;
#if VIRGL_VERSION_MAJOR >= 1
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
@@ -171,6 +171,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState
*qdev)
}
gl->renderer_state = RS_START;
+
+g_array_unref(g->capset_ids);
}
static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index f6cb4fe5b28e..58693dfa2afa 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -629,19 +629,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
VIRTIO_GPU_FILL_CMD(info);
memset(&resp, 0, sizeof(resp));
-if (info.capset_index == 0) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
-virgl_renderer_get_cap_set(resp.capset_id,
- &resp.capset_max_version,
- &resp.capset_max_size);
-} else if (info.capset_index == 1) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
+
+if (info.capset_index < g->capset_ids->len) {
+resp.capset_id = g_array_index(g->capset_ids, uint32_t,
+ info.capset_index);
virgl_renderer_get_cap_set(resp.capset_id,
&resp.capset_max_version,
&resp.capset_max_size);
-} else {
-resp.capset_max_version = 0;
-resp.capset_max_size = 0;
}
resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO;
virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
@@ -1167,12 +1161,27 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
return 0;
}
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
+static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id)
+{
+g_array_append_val(capset_ids, capset_id);
+}
+
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
{
uint32_t capset2_max_ver, capset2_max_size;
+GArray *capset_ids;
+
+capset_ids = g_array_new(false, false, sizeof(uint32_t));
+
+/* VIRGL is always supported. */
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
+
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
&capset2_max_ver,
&capset2_max_size);
+if (capset2_max_ver) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
+}
-return capset2_max_ver ? 2 : 1;
+return capset_ids;
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 775005abb337..83232f4b4bfa 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -209,6 +209,8 @@ struct VirtIOGPU {
QTAILQ_HEAD(, VGPUDMABuf) bufs;
VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
} dmabuf;
+
+GArray *capset_ids;
};
struct VirtIOGPUClass {
@@ -354,6 +356,6 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
#endif
--
2.45.2
[PATCH v15 01/14] virtio-gpu: Use trace events for tracking number of in-flight fences
Replace printf's used for tracking of in-flight fence inc/dec events
with tracing, for consistency with the rest of virtio-gpu code that
uses tracing.
Suggested-by: Marc-André Lureau
Signed-off-by: Dmitry Osipenko
---
hw/display/trace-events | 2 ++
hw/display/virtio-gpu-virgl.c | 2 +-
hw/display/virtio-gpu.c | 4 ++--
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 781f8a33203b..e212710284ae 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -53,6 +53,8 @@ virtio_gpu_cmd_ctx_submit(uint32_t ctx, uint32_t size) "ctx
0x%x, size %d"
virtio_gpu_update_cursor(uint32_t scanout, uint32_t x, uint32_t y, const char
*type, uint32_t res) "scanout %d, x %d, y %d, %s, res 0x%x"
virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ",
type 0x%x"
virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64
+virtio_gpu_inc_inflight_fences(uint32_t inflight) "in-flight+ %u"
+virtio_gpu_dec_inflight_fences(uint32_t inflight) "in-flight- %u"
# qxl.c
disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t
val) "%d %s addr=%u val=%u"
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9f34d0e6619c..14091b191ec0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -525,7 +525,7 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
g_free(cmd);
g->inflight--;
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
+trace_virtio_gpu_dec_inflight_fences(g->inflight);
}
}
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index d60b1b2973af..602952a7041b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1066,7 +1066,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
if (g->stats.max_inflight < g->inflight) {
g->stats.max_inflight = g->inflight;
}
-fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
+trace_virtio_gpu_inc_inflight_fences(g->inflight);
}
} else {
g_free(cmd);
@@ -1086,7 +1086,7 @@ static void virtio_gpu_process_fenceq(VirtIOGPU *g)
g_free(cmd);
g->inflight--;
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
+trace_virtio_gpu_dec_inflight_fences(g->inflight);
}
}
}
--
2.45.2
[PATCH v15 00/14] Support blob memory and venus on qemu
Hello, This series enables Vulkan Venus context support on virtio-gpu. All virglrender and almost all Linux kernel prerequisite changes needed by Venus are already in upstream. For kernel there is a pending KVM patchset that fixes mapping of compound pages needed for DRM drivers using TTM [1], othewrwise hostmem blob mapping will fail with a KVM error from Qemu. [1] https://lore.kernel.org/kvm/20240229025759.1187910-1-steve...@google.com/ You'll need to use recent Mesa version containing patch that removes dependency on cross-device feature from Venus that isn't supported by Qemu [2]. [2] https://gitlab.freedesktop.org/mesa/mesa/-/commit/087e9a96d13155e26987befae78b6ccbb7ae242b Example Qemu cmdline that enables Venus: qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \ -machine q35,accel=kvm,memory-backend=mem1 \ -object memory-backend-memfd,id=mem1,size=8G -m 8G Changes from V14 to V15 - Dropped hostmem mapping state that got unused in v14, suggested by Akihiko Odaki. - Moved resource_get_info() from set_scanout_blob() to create_blob(), suggested by Akihiko Odaki. - Fixed unitilized variable in create_blob(), spotted by Alex Bennée. Changes from V13 to V14 - Fixed erronous fall-through in renderer_state's switch-case that was spotted by Marc-André Lureau. - Reworked HOSTMEM_MR_FINISH_UNMAPPING handling as was suggested by Akihiko Odaki. Now it shares the same code path with HOSTMEM_MR_MAPPED. - Made use of g_autofree in virgl_cmd_resource_create_blob() as was suggested by Akihiko Odaki. - Removed virtio_gpu_virgl_deinit() and moved all deinit code to virtio_gpu_gl_device_unrealize() as was suggested by Marc-André Lureau. - Replaced HAVE_FEATURE in mseon.build with virglrenderer's VERSION_MAJOR check as was suggested by Marc-André Lureau. - Added trace event for cmd-suspension as was suggested by Marc-André Lureau. - Added patch to replace in-flight printf's with trace events as was suggested by Marc-André Lureau Changes from V12 to V13 - Replaced `res->async_unmap_in_progress` flag with a mapping state, moved it to the virtio_gpu_virgl_hostmem_region like was suggested by Akihiko Odaki. - Renamed blob_unmap function and added back cmd_suspended argument to it. Suggested by Akihiko Odaki. - Reordered VirtIOGPUGL refactoring patches to minimize code changes like was suggested by Akihiko Odaki. - Replaced gl->renderer_inited with gl->renderer_state, like was suggested by Alex Bennée. - Added gl->renderer state resetting to gl_device_unrealize(), for consistency. Suggested by Alex Bennée. - Added rb's from Alex and Manos. - Fixed compiling with !HAVE_VIRGL_RESOURCE_BLOB. Changes from V11 to V12 - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't corrupt resource list and releases resource properly on error. Thanks to Akihiko Odaki for spotting the bug. - Added new patch that handles virtio_gpu_virgl_init() failure gracefully, fixing Qemu crash. Besides fixing the crash, it allows to implement a cleaner virtio_gpu_virgl_deinit(). - virtio_gpu_virgl_deinit() now assumes that previously virgl was initialized successfully when it was inited at all. Suggested by Akihiko Odaki. - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit() - Added back blob unmapping or RESOURCE_UNREF that was requested by Akihiko Odaki. Added comment to the code explaining how async unmapping works. Added back `res->async_unmap_in_progress` flag and added comment telling why it's needed. - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes suggested by Akihiko Odaki. - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL for consistency with cmdq_resume_bh. Changes from V10 to V11 - Replaced cmd_resume bool in struct ctrl_command with "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested by Akihiko Odaki. - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding the 'async_unmap_in_progress' flag that was dropped in v9: 1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether resource was previously mapped and lets virglrenderer to do the checking. 2. error returned by virgl_renderer_resource_unmap() is now handled and reported properly, previously the error wasn't checked. The virgl_renderer_resource_unmap() fails if resource wasn't mapped. 3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource that is mapped, it's a error condition if guest didn't unmap resource before doing the unref. Previously unref was implicitly unmapping resource. Changes from V9 to V10 - Dropped 'async_unmap_in_progress' variable and switched to use aio_bh_new() isntead of oneshot variant in the "blob commands" patch. - Further improved error messages by printing error code when actual error occurrs and using ERR_UNSPEC instead of ERR_ENOME
[PATCH] util: fix building on OpenBSD/powerpc
util: fix building on OpenBSD/powerpc
Signed-off-by: Brad Smith
---
util/cpuinfo-ppc.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/util/cpuinfo-ppc.c b/util/cpuinfo-ppc.c
index b2d8893a06..d459c9c87e 100644
--- a/util/cpuinfo-ppc.c
+++ b/util/cpuinfo-ppc.c
@@ -6,11 +6,13 @@
#include "qemu/osdep.h"
#include "host/cpuinfo.h"
-#include
-#ifdef CONFIG_GETAUXVAL
-# include
-#else
-# include "elf.h"
+#ifdef CONFIG_LINUX
+# ifdef CONFIG_GETAUXVAL
+# include
+# else
+# include
+# include "elf.h"
+# endif
#endif
unsigned cpuinfo;
@@ -19,16 +21,17 @@ unsigned cpuinfo;
unsigned __attribute__((constructor)) cpuinfo_init(void)
{
unsigned info = cpuinfo;
-unsigned long hwcap, hwcap2;
if (info) {
return info;
}
-hwcap = qemu_getauxval(AT_HWCAP);
-hwcap2 = qemu_getauxval(AT_HWCAP2);
info = CPUINFO_ALWAYS;
+#ifdef CONFIG_LINUX
+unsigned long hwcap = qemu_getauxval(AT_HWCAP);
+unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2);
+
/* Version numbers are monotonic, and so imply all lower versions. */
if (hwcap2 & PPC_FEATURE2_ARCH_3_1) {
info |= CPUINFO_V3_1 | CPUINFO_V3_0 | CPUINFO_V2_07 | CPUINFO_V2_06;
@@ -58,6 +61,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
}
}
}
+#endif
cpuinfo = info;
return info;
--
2.45.2
[PATCH] util/cpuinfo-aarch64: Add OpenBSD support
util/cpuinfo-aarch64: Add OpenBSD support
Signed-off-by: Brad Smith
---
util/cpuinfo-aarch64.c | 32
1 file changed, 32 insertions(+)
diff --git a/util/cpuinfo-aarch64.c b/util/cpuinfo-aarch64.c
index 4c8a005715..8a8c0a30a8 100644
--- a/util/cpuinfo-aarch64.c
+++ b/util/cpuinfo-aarch64.c
@@ -20,6 +20,12 @@
#ifdef CONFIG_DARWIN
# include
#endif
+#ifdef __OpenBSD__
+# include
+# include
+# include
+# include
+#endif
unsigned cpuinfo;
@@ -72,6 +78,32 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
info |= sysctl_for_bool("hw.optional.arm.FEAT_PMULL") * CPUINFO_PMULL;
info |= sysctl_for_bool("hw.optional.arm.FEAT_BTI") * CPUINFO_BTI;
#endif
+#ifdef __OpenBSD__
+int mib[2];
+uint64_t isar0;
+uint64_t pfr1;
+size_t len;
+
+mib[0] = CTL_MACHDEP;
+mib[1] = CPU_ID_AA64ISAR0;
+len = sizeof(isar0);
+if (sysctl(mib, 2, &isar0, &len, NULL, 0) != -1) {
+ if (ID_AA64ISAR0_ATOMIC(isar0) >= ID_AA64ISAR0_ATOMIC_IMPL)
+info |= CPUINFO_LSE;
+ if (ID_AA64ISAR0_AES(isar0) >= ID_AA64ISAR0_AES_BASE)
+info |= CPUINFO_AES;
+ if (ID_AA64ISAR0_AES(isar0) >= ID_AA64ISAR0_AES_PMULL)
+info |= CPUINFO_PMULL;
+}
+
+mib[0] = CTL_MACHDEP;
+mib[1] = CPU_ID_AA64PFR1;
+len = sizeof(pfr1);
+if (sysctl(mib, 2, &pfr1, &len, NULL, 0) != -1) {
+ if (ID_AA64PFR1_BT(pfr1) >= ID_AA64PFR1_BT_IMPL)
+info |= CPUINFO_BTI;
+}
+#endif
cpuinfo = info;
return info;
--
2.45.2
[PATCH] hw/ufs: Fix potential bugs in MMIO read|write
This patch fixes two points reported in coverity scan report [1]. Check
the MMIO access address with (addr + size), not just with the start offset
addr to make sure that the requested memory access not to exceed the
actual register region. We also updated (uint8_t *) to (uint32_t *) to
represent we are accessing the MMIO registers by dword-sized only.
[1]
https://lore.kernel.org/qemu-devel/cafeaca82l-wznhmw0x+dr40bhm-evq2zh4dg4pdqop4xxdp...@mail.gmail.com/
Cc: Jeuk Kim
Reported-by: Peter Maydell
Signed-off-by: Minwoo Im
---
hw/ufs/ufs.c | 31 ---
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 71a88d221ced..bf2ff02ac6e5 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -55,17 +55,18 @@ static inline uint64_t ufs_reg_size(UfsHc *u)
return ufs_mcq_op_reg_addr(u, 0) + sizeof(u->mcq_op_reg);
}
-static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr)
+static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr, unsigned size)
{
uint64_t mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
-return addr >= mcq_reg_addr && addr < mcq_reg_addr + sizeof(u->mcq_reg);
+return (addr >= mcq_reg_addr &&
+addr + size <= mcq_reg_addr + sizeof(u->mcq_reg));
}
-static inline bool ufs_is_mcq_op_reg(UfsHc *u, uint64_t addr)
+static inline bool ufs_is_mcq_op_reg(UfsHc *u, uint64_t addr, unsigned size)
{
uint64_t mcq_op_reg_addr = ufs_mcq_op_reg_addr(u, 0);
return (addr >= mcq_op_reg_addr &&
-addr < mcq_op_reg_addr + sizeof(u->mcq_op_reg));
+addr + size <= mcq_op_reg_addr + sizeof(u->mcq_op_reg));
}
static MemTxResult ufs_addr_read(UfsHc *u, hwaddr addr, void *buf, int size)
@@ -774,25 +775,25 @@ static void ufs_write_mcq_op_reg(UfsHc *u, hwaddr offset,
uint32_t data,
static uint64_t ufs_mmio_read(void *opaque, hwaddr addr, unsigned size)
{
UfsHc *u = (UfsHc *)opaque;
-uint8_t *ptr;
+uint32_t *ptr;
uint64_t value;
uint64_t offset;
-if (addr < sizeof(u->reg)) {
+if (addr + size <= sizeof(u->reg)) {
offset = addr;
-ptr = (uint8_t *)&u->reg;
-} else if (ufs_is_mcq_reg(u, addr)) {
+ptr = (uint32_t *)&u->reg;
+} else if (ufs_is_mcq_reg(u, addr, size)) {
offset = addr - ufs_mcq_reg_addr(u, 0);
-ptr = (uint8_t *)&u->mcq_reg;
-} else if (ufs_is_mcq_op_reg(u, addr)) {
+ptr = (uint32_t *)&u->mcq_reg;
+} else if (ufs_is_mcq_op_reg(u, addr, size)) {
offset = addr - ufs_mcq_op_reg_addr(u, 0);
-ptr = (uint8_t *)&u->mcq_op_reg;
+ptr = (uint32_t *)&u->mcq_op_reg;
} else {
trace_ufs_err_invalid_register_offset(addr);
return 0;
}
-value = *(uint32_t *)(ptr + offset);
+value = ptr[offset >> 2];
trace_ufs_mmio_read(addr, value, size);
return value;
}
@@ -804,11 +805,11 @@ static void ufs_mmio_write(void *opaque, hwaddr addr,
uint64_t data,
trace_ufs_mmio_write(addr, data, size);
-if (addr < sizeof(u->reg)) {
+if (addr + size <= sizeof(u->reg)) {
ufs_write_reg(u, addr, data, size);
-} else if (ufs_is_mcq_reg(u, addr)) {
+} else if (ufs_is_mcq_reg(u, addr, size)) {
ufs_write_mcq_reg(u, addr - ufs_mcq_reg_addr(u, 0), data, size);
-} else if (ufs_is_mcq_op_reg(u, addr)) {
+} else if (ufs_is_mcq_op_reg(u, addr, size)) {
ufs_write_mcq_op_reg(u, addr - ufs_mcq_op_reg_addr(u, 0), data, size);
} else {
trace_ufs_err_invalid_register_offset(addr);
--
2.34.1
Re: [PATCH v15 10/14] virtio-gpu: Support blob scanout using dmabuf fd
On 2024/06/23 6:55, Dmitry Osipenko wrote:
From: Robert Beckett
Support displaying blob resources by handling SET_SCANOUT_BLOB
command.
Signed-by: Antonio Caggiano
Signed-off-by: Robert Beckett
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 109 +
hw/display/virtio-gpu.c| 12 ++--
include/hw/virtio/virtio-gpu.h | 7 +++
3 files changed, 122 insertions(+), 6 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 3ffea478e723..60befab7efc2 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -17,6 +17,8 @@
#include "trace.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-gpu-pixman.h"
#include "ui/egl-helpers.h"
@@ -78,6 +80,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
res->base.height = c2d.height;
res->base.format = c2d.format;
res->base.resource_id = c2d.resource_id;
+res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c2d.resource_id;
@@ -125,6 +128,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
res->base.height = c3d.height;
res->base.format = c3d.format;
res->base.resource_id = c3d.resource_id;
+res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c3d.resource_id;
@@ -509,6 +513,106 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
g_free(resp);
}
+#if VIRGL_VERSION_MAJOR >= 1
+static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_framebuffer fb = { 0 };
+struct virgl_renderer_resource_info info;
+struct virtio_gpu_virgl_resource *res;
+struct virtio_gpu_set_scanout_blob ss;
+uint64_t fbend;
+
+VIRTIO_GPU_FILL_CMD(ss);
+virtio_gpu_scanout_blob_bswap(&ss);
+trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
+ ss.r.width, ss.r.height, ss.r.x,
+ ss.r.y);
+
+if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
+ __func__, ss.scanout_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+return;
+}
+
+if (ss.resource_id == 0) {
+virtio_gpu_disable_scanout(g, ss.scanout_id);
+return;
+}
+
+if (ss.width < 16 ||
+ss.height < 16 ||
+ss.r.x + ss.r.width > ss.width ||
+ss.r.y + ss.r.height > ss.height) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
+ " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
+ __func__, ss.scanout_id, ss.resource_id,
+ ss.r.x, ss.r.y, ss.r.width, ss.r.height,
+ ss.width, ss.height);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, ss.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (virgl_renderer_resource_get_info(ss.resource_id, &info)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not have info %d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (res->base.dmabuf_fd < 0) {
+res->base.dmabuf_fd = info.fd;
Just in case you missed my previous email:
> res->base.dmabuf_fd is conditionally assigned but
> virgl_renderer_resource_get_info() is called unconditionally, which is
> inconsistent.
> The relevant code is better to be moved into
> virgl_cmd_resource_create_blob() for consistenty with
> virtio_gpu_resource_create_blob().
Re: [PATCH v15 12/14] virtio-gpu: Handle resource blob commands
On 2024/06/23 6:55, Dmitry Osipenko wrote:
From: Antonio Caggiano
Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true
Signed-off-by: Antonio Caggiano
Signed-off-by: Xenia Ragiadakou
Signed-off-by: Huang Rui
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 3 +
hw/display/virtio-gpu-virgl.c | 334 +++--
hw/display/virtio-gpu.c| 6 +-
include/hw/virtio/virtio-gpu.h | 2 +
4 files changed, 330 insertions(+), 15 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 4fe9e6a0c21c..5f27568d3ec8 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -160,6 +160,9 @@ static void virtio_gpu_gl_device_unrealize(DeviceState
*qdev)
VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
if (gl->renderer_state >= RS_INITED) {
+#if VIRGL_VERSION_MAJOR >= 1
+qemu_bh_delete(gl->cmdq_resume_bh);
+#endif
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
timer_free(gl->print_stats);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 60befab7efc2..f6cb4fe5b28e 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -26,6 +26,7 @@
struct virtio_gpu_virgl_resource {
struct virtio_gpu_simple_resource base;
+MemoryRegion *mr;
};
static struct virtio_gpu_virgl_resource *
@@ -49,6 +50,152 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
}
#endif
+#if VIRGL_VERSION_MAJOR >= 1
+typedef enum {
+HOSTMEM_MR_UNMAPPING,
+HOSTMEM_MR_FINISH_UNMAPPING,
+} HostmemMRState;
Now you can make it a mere bool.
Re: [PATCH v15 00/14] Support blob memory and venus on qemu
On 2024/06/23 6:54, Dmitry Osipenko wrote: Hello, This series enables Vulkan Venus context support on virtio-gpu. Thanks again for keeping working on this. This series became quite a mature. I still have comments for two patches, but they are trivial ones so I hope it won't take much time to fix them. By the way, the rutabaga patch series added the documentation for virtio-gpu at docs/system/devices/virtio-gpu.rst. It does not only cover rutabaga but also virgl, and says virglrenderer translates OpenGL calls, which becomes somewhat misleading after this patch. Please update it to tell that it can also pass-through Vulkan calls when Venus enabled.
