Re: [PATCH] ui/cocoa: Fixed modeswitching glitch in zoomed fullscreen mode
On 2022/06/19 9:41, Imran Yusuff wrote:
This applies only on macOS using cocoa UI library.
In zoom-to-fit fullscreen mode, upon graphics mode switch,
the viewport size is wrong, and the usual consequence
is only a part of the screen is visible. One have to exit
and reenter fullscreen mode to fix this.
This is reproducible by setting up a Windows 3.11 system,
booting into DOS, enable zoom-to-fit, enter fullscreen mode and
start Windows by 'win'. Then you can see only part of the screen.
This commit fixes this problem, by including one line of code
which is from the fullscreen mode initialization.
Signed-off-by: Imran Yusuff
---
ui/cocoa.m | 1 +
1 file changed, 1 insertion(+)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 84c84e98fc..bd602817cd 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -636,6 +636,7 @@ - (void) switchSurface:(pixman_image_t *)image
if (isFullscreen) {
[[fullScreenWindow contentView] setFrame:[[NSScreen mainScreen]
frame]];
[normalWindow setFrame:NSMakeRect([normalWindow frame].origin.x,
[normalWindow frame].origin.y - h + oldh, w, h + [normalWindow
frame].size.height - oldh) display:NO animate:NO];
+[self setFrame:NSMakeRect(cx, cy, cw, ch)];
} else {
if (qemu_name)
[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s",
qemu_name]];
[self setFrame:NSMakeRect(cx, cy, cw, ch)] already exist in the earlier
part of the method. It is redundant and not a proper way to fix. It is
necessary to understand why adding the same statement fixes the problem,
and to fix it without duplicate statements. I can think of two
possibilities:
1. The isResize variable is not enough to cover all the situations when
the statement needs to be executed.
2. The statement needs to be executed after [normalWindow setFrame:].
In case 1, you need to assign a correct value to isResize or add [self
setFrame:] to somewhere else, but not in [-CocoaView switchSurface].
Putting the statement in [-CocoaView switchSurface] may cause redundant
execution of the statement when isResize is true as I explained.
In case 2, the existing statement should be simply moved.
By the way, I have a patch to rewrite the code implementing full screen
so you may try it. If you tested the patch, please give Tested-by.
https://patchew.org/QEMU/20220316060244.46669-1-akihiko.od...@gmail.com/
Regards,
Akihiko Odaki
TCG development tools?
Hello all, Suppose I am wanting to implement and test for the TCG. Is there any set of interactive binaries or other things that I could use to work with it? Could I open a shell to it? Perhaps feed it specific byte sequences for tests to ensure correct operation? Are there a canonical set of unit tests that exercise the TCG that each architecture implements?
[PATCH 2/2] target/arm: Check V7VE as well as LPAE in arm_pamax
In machvirt_init we create a cpu but do not fully initialize it.
Thus the propagation of V7VE to LPAE has not been done, and we
compute the wrong value for some v7 cpus, e.g. cortex-a15.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1078
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 07f7a21861..da478104f0 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -47,7 +47,13 @@ unsigned int arm_pamax(ARMCPU *cpu)
assert(parange < ARRAY_SIZE(pamax_map));
return pamax_map[parange];
}
-if (arm_feature(>env, ARM_FEATURE_LPAE)) {
+
+/*
+ * In machvirt_init, we call arm_pamax on a cpu that is not fully
+ * initialized, so we can't rely on the propagation done in realize.
+ */
+if (arm_feature(>env, ARM_FEATURE_LPAE) ||
+arm_feature(>env, ARM_FEATURE_V7VE)) {
/* v7 with LPAE */
return 40;
}
--
2.34.1
[PATCH 0/2] target/arm: Fix issue 1078
Nicely summarized by the reporter, but I thought it would be nicer to pull all of the logic into arm_pamax, rather than leave it separated. r~ Richard Henderson (2): target/arm: Extend arm_pamax to more than aarch64 target/arm: Check V7VE as well as LPAE in arm_pamax hw/arm/virt.c| 10 +- target/arm/ptw.c | 26 -- 2 files changed, 21 insertions(+), 15 deletions(-) -- 2.34.1
[PATCH 1/2] target/arm: Extend arm_pamax to more than aarch64
Move the code from hw/arm/virt.c that is supposed
to handle v7 into the one function.
Signed-off-by: Richard Henderson
---
hw/arm/virt.c| 10 +-
target/arm/ptw.c | 24
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 097238faa7..5502aa60c8 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2010,15 +2010,7 @@ static void machvirt_init(MachineState *machine)
cpuobj = object_new(possible_cpus->cpus[0].type);
armcpu = ARM_CPU(cpuobj);
-if (object_property_get_bool(cpuobj, "aarch64", NULL)) {
-pa_bits = arm_pamax(armcpu);
-} else if (arm_feature(>env, ARM_FEATURE_LPAE)) {
-/* v7 with LPAE */
-pa_bits = 40;
-} else {
-/* Anything else */
-pa_bits = 32;
-}
+pa_bits = arm_pamax(armcpu);
object_unref(cpuobj);
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 4d97a24808..07f7a21861 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -36,15 +36,23 @@ static const uint8_t pamax_map[] = {
/* The cpu-specific constant value of PAMax; also used by hw/arm/virt. */
unsigned int arm_pamax(ARMCPU *cpu)
{
-unsigned int parange =
-FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE);
+if (arm_feature(>env, ARM_FEATURE_AARCH64)) {
+unsigned int parange =
+FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE);
-/*
- * id_aa64mmfr0 is a read-only register so values outside of the
- * supported mappings can be considered an implementation error.
- */
-assert(parange < ARRAY_SIZE(pamax_map));
-return pamax_map[parange];
+/*
+ * id_aa64mmfr0 is a read-only register so values outside of the
+ * supported mappings can be considered an implementation error.
+ */
+assert(parange < ARRAY_SIZE(pamax_map));
+return pamax_map[parange];
+}
+if (arm_feature(>env, ARM_FEATURE_LPAE)) {
+/* v7 with LPAE */
+return 40;
+}
+/* Anything else */
+return 32;
}
/*
--
2.34.1
[PATCH 3/4] device_tree: add qemu_fdt_setprop_strings() helper
Add a helper to set a property from a set of strings
to reduce the following code:
static const char * const clint_compat[2] = {
"sifive,clint0", "riscv,clint0"
};
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
Signed-off-by: Ben Dooks
--
v2:
- fix node/path in comment
---
include/sysemu/device_tree.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 79ce009a22..28b68bacaf 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,21 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
const char *prop, char **array, int len);
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @path: node name
+ * @prop: property array
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...) \
+do {\
+static const char * const __strs[] = { __VA_ARGS__ }; \
+qemu_fdt_setprop_string_array(fdt, path, prop, \
+(char **)&__strs, ARRAY_SIZE(__strs)); \
+} while(0)
+
+
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
const char *property,
const char *target_node_path);
--
2.35.1
[v2] pair of device-tree helpers
I've been doing a bit of looking at riscv and dt creation, and was thinking the following two helper functions would be useful so implemented qemu_fdt_setprop_reg64_map() and qemu_fdt_setprop_strings() and then applied them to the hw/riscv/sifive_u.c machine.
[PATCH 1/4] device_tree: add qemu_fdt_setprop_reg64_map helper
Add a macro qemu_fdt_setprop_reg64_map() to set the given node's reg property directly from the memory map entry to avoid open coding of the following: qemu_fdt_setprop_cells(fdt, nodename, "reg", 0x0, memmap[SIFIVE_U_DEV_OTP].base, 0x0, memmap[SIFIVE_U_DEV_OTP].size); Signed-off-by: Ben Dooks -- v2: - changed qemu_fdt_setprop_reg64 to qemu_fdt_setprop_reg64_map --- include/sysemu/device_tree.h | 15 +++ 1 file changed, 15 insertions(+) diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index ef060a9759..79ce009a22 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -135,6 +135,21 @@ int qemu_fdt_add_path(void *fdt, const char *path); sizeof(qdt_tmp));\ } while (0) +/** + * qemu_fdt_setprop_reg64_map: + * @fdt: the device tree path + * @node_path: node to set property on + * @map: the map entry to set the reg from + * + * A helper tp set the 'reg' node on the specified node from the given map + * entry. + */ +#define qemu_fdt_setprop_reg64_map(fdt, path, map) \ +qemu_fdt_setprop_cells(fdt, path, "reg",\ + (map)->base >> 32, (map)->base, \ + (map)->size >> 32, (map)->size) + + void qemu_fdt_dumpdtb(void *fdt, int size); /** -- 2.35.1
[PATCH 2/4] hw/riscv: use qemu_fdt_setprop_reg64_map() in sifive_u.c
Use the qemu_fdt_setprop_reg64_map() to replace the code
that sets the property manually.
Signed-off-by: Ben Dooks
--
v2:
- changed to qemu_fdt_setprop_reg64_map() from previous
---
hw/riscv/sifive_u.c | 41 +++--
1 file changed, 11 insertions(+), 30 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..89d7aa2a52 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -223,9 +223,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
(char **)_compat, ARRAY_SIZE(clint_compat));
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_CLINT].base,
-0x0, memmap[SIFIVE_U_DEV_CLINT].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
g_free(cells);
@@ -235,9 +233,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_OTP].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "fuse-count", SIFIVE_U_OTP_REG_SIZE);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_OTP].base,
-0x0, memmap[SIFIVE_U_DEV_OTP].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_OTP]);
qemu_fdt_setprop_string(fdt, nodename, "compatible",
"sifive,fu540-c000-otp");
g_free(nodename);
@@ -250,9 +246,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "#clock-cells", 0x1);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
hfclk_phandle, rtcclk_phandle);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_PRCI].base,
-0x0, memmap[SIFIVE_U_DEV_PRCI].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_PRCI]);
qemu_fdt_setprop_string(fdt, nodename, "compatible",
"sifive,fu540-c000-prci");
g_free(nodename);
@@ -284,9 +278,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_PLIC].base,
-0x0, memmap[SIFIVE_U_DEV_PLIC].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_PLIC]);
qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35);
qemu_fdt_setprop_cell(fdt, nodename, "phandle", plic_phandle);
plic_phandle = qemu_fdt_get_phandle(fdt, nodename);
@@ -304,9 +296,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop_cell(fdt, nodename, "#gpio-cells", 2);
qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_GPIO].base,
-0x0, memmap[SIFIVE_U_DEV_GPIO].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_GPIO]);
qemu_fdt_setprop_cells(fdt, nodename, "interrupts", SIFIVE_U_GPIO_IRQ0,
SIFIVE_U_GPIO_IRQ1, SIFIVE_U_GPIO_IRQ2, SIFIVE_U_GPIO_IRQ3,
SIFIVE_U_GPIO_IRQ4, SIFIVE_U_GPIO_IRQ5, SIFIVE_U_GPIO_IRQ6,
@@ -342,9 +332,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/cache-controller@%lx",
(long)memmap[SIFIVE_U_DEV_L2CC].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_L2CC].base,
-0x0, memmap[SIFIVE_U_DEV_L2CC].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_L2CC]);
qemu_fdt_setprop_cells(fdt, nodename, "interrupts",
SIFIVE_U_L2CC_IRQ0, SIFIVE_U_L2CC_IRQ1, SIFIVE_U_L2CC_IRQ2);
qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
@@ -366,9 +354,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
prci_phandle, PRCI_CLK_TLCLK);
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI2_IRQ);
qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
-qemu_fdt_setprop_cells(fdt, nodename, "reg",
-0x0, memmap[SIFIVE_U_DEV_QSPI2].base,
-0x0, memmap[SIFIVE_U_DEV_QSPI2].size);
+qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_QSPI2]);
qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,spi0");
g_free(nodename);
@@ -391,9 +377,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
prci_phandle, PRCI_CLK_TLCLK);
[PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify
the code.
Signed-off-by: Ben Dooks
---
hw/riscv/sifive_u.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 89d7aa2a52..16b18d90bd 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-static const char * const ethclk_names[2] = { "pclk", "hclk" };
-static const char * const clint_compat[2] = {
-"sifive,clint0", "riscv,clint0"
-};
-static const char * const plic_compat[2] = {
-"sifive,plic-1.0.0", "riscv,plic0"
-};
if (ms->dtb) {
fdt = s->fdt = load_device_tree(ms->dtb, >fdt_size);
@@ -221,8 +214,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
nodename = g_strdup_printf("/soc/clint@%lx",
(long)memmap[SIFIVE_U_DEV_CLINT].base);
qemu_fdt_add_subnode(fdt, nodename);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(clint_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+ "sifive,clint0", "riscv,clint0");
qemu_fdt_setprop_reg64_map(fdt, nodename, [SIFIVE_U_DEV_CLINT]);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, ms->smp.cpus * sizeof(uint32_t) * 4);
@@ -273,8 +266,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
(long)memmap[SIFIVE_U_DEV_PLIC].base);
qemu_fdt_add_subnode(fdt, nodename);
qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-(char **)_compat, ARRAY_SIZE(plic_compat));
+qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+ "sifive,plic-1.0.0", "riscv,plic0");
qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -410,8 +403,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
qemu_fdt_setprop_cells(fdt, nodename, "clocks",
prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-(char **)_names, ARRAY_SIZE(ethclk_names));
+qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
qemu_fdt_setprop(fdt, nodename, "local-mac-address",
s->soc.gem.conf.macaddr.a, ETH_ALEN);
qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
--
2.35.1
Re: [PATCH 4/4] hw/riscv: use qemu_fdt_setprop_strings() in sifive_u.c
On Fri, Apr 22, 2022 at 10:19:34AM +0800, Bin Meng wrote: > On Mon, Apr 18, 2022 at 5:13 AM Ben Dooks wrote: > > > > Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify > > the code. > > > > Signed-off-by; Ben Dooks > > ; should be replaced to : > > Not sure how you did that, but you can do with "git commit -s" and git > will take care of the SoB tag. I'm used to adding them manually to git commit messages. -- Ben Dooks, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.
Re: [PATCH v2] target/ppc: cpu_init: Clean up stop state on cpu reset
On 6/17/22 11:52, Frederic Barrat wrote: The 'resume_as_sreset' attribute of a cpu is set when a thread is entering a stop state on ppc books. It causes the thread to be re-routed to vector 0x100 when woken up by an exception. So it must be cleared on reset or a thread might be re-routed unexpectedly after a reset, when it was not in a stop state and/or when the appropriate exception handler isn't set up yet. Using skiboot, it can be tested by resetting the system when it is quiet and most threads are idle and in stop state. After the reset occurs, skiboot elects a primary thread and all the others wait in secondary_wait. The primary thread does all the system initialization from main_cpu_entry() and at some point, the decrementer interrupt starts ticking. The exception vector for the decrementer interrupt is in place, so that shouldn't be a problem. However, if that primary thread was in stop state prior to the reset, and because the resume_as_sreset parameters is still set, it is re-routed to exception vector 0x100. Which, at that time, is still defined as the entry point for BML. So that primary thread restarts as new and ends up being treated like any other secondary thread. All threads are now waiting in secondary_wait. It results in a full system hang with no message on the console, as the uart hasn't been init'ed yet. It's actually not obvious to realise what's happening if not tracing reset (-d cpu_reset). The fix is simply to clear the 'resume_as_sreset' attribute on reset. Signed-off-by: Frederic Barrat --- Changelog: v2: rework commit message Nice ! This has been a long standing bug. I chased it for weeks. I was reproducing with intensive I/Os, doing an scp on an emulated PowerNV machine. It hung after a while (unless using powersave=off) Now, with this patch, a QEMU PowerNV POWER9 machine (SMP) running a Linux 5.18 sustains the load : $ scp ./ubuntu-22.04-ppc64le.qcow2 root@vm103:/dev/null root@vm103's password: ubuntu-22.04-ppc64le.qcow2100% 8581MB 5.8MB/s 24:39 Quite a few interrupts : # grep PNV-PCI-MSI /proc/interrupts 51: 9 0 PNV-PCI-MSI 403177472 Edge nvme0q0 52: 2 0 PNV-PCI-MSI 403177473 Edge nvme0q1 53: 0 0 PNV-PCI-MSI 403177474 Edge nvme0q2 54:3427556 0 PNV-PCI-MSI 135315456 Edge eth0-rx-0 55: 04261742 PNV-PCI-MSI 135315457 Edge eth0-tx-0 56: 1 0 PNV-PCI-MSI 135315458 Edge eth0 57: 0 71 PNV-PCI-MSI 135299072 Edge xhci_hcd 58: 0 0 PNV-PCI-MSI 135299073 Edge xhci_hcd 59: 0 0 PNV-PCI-MSI 135299074 Edge xhci_hcd It would be nice to explain what you did to corner the issue. It would help other people chasing similar bugs in QEMU or in the kernel. Thanks, C. target/ppc/cpu_init.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index 0f891afa04..c16cb8dbe7 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -7186,6 +7186,9 @@ static void ppc_cpu_reset(DeviceState *dev) } pmu_update_summaries(env); } + +/* clean any pending stop state */ +env->resume_as_sreset = 0; #endif hreg_compute_hflags(env); env->reserve_addr = (target_ulong)-1ULL;
[PATCH v2 9/9] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb4 root bus
It's unneeded. No other PCIE_BUS implements this interface.
Fixes: 4f9924c4d4cf ("ppc/pnv: Add models for POWER9 PHB4 PCIe Host bridge")
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb4.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 61b45fe33c..81f7c1fe8f 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1751,10 +1751,6 @@ static const TypeInfo pnv_phb4_root_bus_info = {
.name = TYPE_PNV_PHB4_ROOT_BUS,
.parent = TYPE_PCIE_BUS,
.class_init = pnv_phb4_root_bus_class_init,
-.interfaces = (InterfaceInfo[]) {
-{ INTERFACE_PCIE_DEVICE },
-{ }
-},
};
static void pnv_phb4_root_port_reset(DeviceState *dev)
--
2.36.1
[PATCH v2 5/9] ppc/pnv: make pnv_ics_get() use the chip8->phbs[] array
The function is working today by getting all the child objects of the
chip, interacting with each of them to check whether the child is a PHB,
and then doing what needs to be done.
We have all the chip PHBs in the phbs[] array so interacting with all
child objects is unneeded. Open code pnv_ics_get_phb_ics() into
pnv_ics_get() and remove both pnv_ics_get_phb_ics() and the
ForeachPhb3Args struct.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/pnv.c | 38 +++---
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 40e0cbd84d..ff7f803662 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1939,44 +1939,28 @@ PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t
pir)
return NULL;
}
-typedef struct ForeachPhb3Args {
-int irq;
-ICSState *ics;
-} ForeachPhb3Args;
-
-static int pnv_ics_get_child(Object *child, void *opaque)
-{
-ForeachPhb3Args *args = opaque;
-PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
-
-if (phb3) {
-if (ics_valid_irq(>lsis, args->irq)) {
-args->ics = >lsis;
-}
-if (ics_valid_irq(ICS(>msis), args->irq)) {
-args->ics = ICS(>msis);
-}
-}
-return args->ics ? 1 : 0;
-}
-
static ICSState *pnv_ics_get(XICSFabric *xi, int irq)
{
PnvMachineState *pnv = PNV_MACHINE(xi);
-ForeachPhb3Args args = { irq, NULL };
-int i;
+int i, j;
for (i = 0; i < pnv->num_chips; i++) {
-PnvChip *chip = pnv->chips[i];
Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
if (ics_valid_irq(>psi.ics, irq)) {
return >psi.ics;
}
-object_child_foreach(OBJECT(chip), pnv_ics_get_child, );
-if (args.ics) {
-return args.ics;
+for (j = 0; j < chip8->num_phbs; j++) {
+PnvPHB3 *phb3 = >phbs[j];
+
+if (ics_valid_irq(>lsis, irq)) {
+return >lsis;
+}
+
+if (ics_valid_irq(ICS(>msis), irq)) {
+return ICS(>msis);
+}
}
}
return NULL;
--
2.36.1
[PATCH v2 4/9] ppc/pnv: use dev instead of pci->qdev in root_port_realize()
We already have access to the 'dev' object.
Reviewed-by: Frederic Barrat
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb3.c | 4 ++--
hw/pci-host/pnv_phb4.c | 5 ++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 5e7f827415..8c03cc94f2 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1157,8 +1157,8 @@ static void pnv_phb3_root_port_realize(DeviceState *dev,
Error **errp)
}
/* Set unique chassis/slot values for the root port */
-qdev_prop_set_uint8(>qdev, "chassis", phb->chip_id);
-qdev_prop_set_uint16(>qdev, "slot", phb->phb_id);
+qdev_prop_set_uint8(dev, "chassis", phb->chip_id);
+qdev_prop_set_uint16(dev, "slot", phb->phb_id);
rpc->parent_realize(dev, _err);
if (local_err) {
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index a0ee52e820..61b45fe33c 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1781,7 +1781,6 @@ static void pnv_phb4_root_port_reset(DeviceState *dev)
static void pnv_phb4_root_port_realize(DeviceState *dev, Error **errp)
{
PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
-PCIDevice *pci = PCI_DEVICE(dev);
PnvPHB4 *phb = NULL;
Error *local_err = NULL;
@@ -1799,8 +1798,8 @@ static void pnv_phb4_root_port_realize(DeviceState *dev,
Error **errp)
}
/* Set unique chassis/slot values for the root port */
-qdev_prop_set_uint8(>qdev, "chassis", phb->chip_id);
-qdev_prop_set_uint16(>qdev, "slot", phb->phb_id);
+qdev_prop_set_uint8(dev, "chassis", phb->chip_id);
+qdev_prop_set_uint16(dev, "slot", phb->phb_id);
rpc->parent_realize(dev, _err);
if (local_err) {
--
2.36.1
[PATCH v2 8/9] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb3 root bus
It's unneeded. No other PCIE_BUS implements this interface.
Fixes: 9ae1329ee2fe ("ppc/pnv: Add models for POWER8 PHB3 PCIe Host bridge")
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb3.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 8c03cc94f2..696c9bef26 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1129,10 +1129,6 @@ static const TypeInfo pnv_phb3_root_bus_info = {
.name = TYPE_PNV_PHB3_ROOT_BUS,
.parent = TYPE_PCIE_BUS,
.class_init = pnv_phb3_root_bus_class_init,
-.interfaces = (InterfaceInfo[]) {
-{ INTERFACE_PCIE_DEVICE },
-{ }
-},
};
static void pnv_phb3_root_port_realize(DeviceState *dev, Error **errp)
--
2.36.1
[PATCH v2 6/9] ppc/pnv: make pnv_ics_resend() use chip8->phbs[]
pnv_ics_resend() is scrolling through all the child objects of the chip
to search for the PHBs. It's faster and simpler to just use the phbs[]
array.
pnv_ics_resend_child() was folded into pnv_ics_resend() since it's too
simple to justify its own function.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/pnv.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index ff7f803662..08136def8e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1979,28 +1979,22 @@ PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t
chip_id)
return NULL;
}
-static int pnv_ics_resend_child(Object *child, void *opaque)
-{
-PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
-
-if (phb3) {
-ics_resend(>lsis);
-ics_resend(ICS(>msis));
-}
-return 0;
-}
-
static void pnv_ics_resend(XICSFabric *xi)
{
PnvMachineState *pnv = PNV_MACHINE(xi);
-int i;
+int i, j;
for (i = 0; i < pnv->num_chips; i++) {
-PnvChip *chip = pnv->chips[i];
Pnv8Chip *chip8 = PNV8_CHIP(pnv->chips[i]);
ics_resend(>psi.ics);
-object_child_foreach(OBJECT(chip), pnv_ics_resend_child, NULL);
+
+for (j = 0; j < chip8->num_phbs; j++) {
+PnvPHB3 *phb3 = >phbs[j];
+
+ics_resend(>lsis);
+ics_resend(ICS(>msis));
+}
}
}
--
2.36.1
[PATCH v2 3/9] ppc/pnv: use dev->parent_bus->parent to get the PHB
It is not advisable to execute an object_dynamic_cast() to poke into
bus->qbus.parent and follow it up with a C cast into the PnvPHB type we
think we got.
A better way is to access the PnvPHB object via a QOM macro accessing
the existing parent links of the DeviceState. For a given
pnv-phb3/4-root-port 'dev', dev->parent_bus will give us the PHB bus,
and dev->parent_bus->parent is the PHB. Use the adequate QOM macro to
assert the type, and keep the NULL check in case we didn't get the
object we were expecting.
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb3.c | 10 +++---
hw/pci-host/pnv_phb4.c | 10 +++---
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 4ba660f8b9..5e7f827415 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1139,12 +1139,16 @@ static void pnv_phb3_root_port_realize(DeviceState
*dev, Error **errp)
{
PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
PCIDevice *pci = PCI_DEVICE(dev);
-PCIBus *bus = pci_get_bus(pci);
PnvPHB3 *phb = NULL;
Error *local_err = NULL;
-phb = (PnvPHB3 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
- TYPE_PNV_PHB3);
+/*
+ * dev->parent_bus gives access to the pnv-phb-root bus.
+ * The PnvPHB3 is the owner (parent) of the bus.
+ */
+if (dev->parent_bus) {
+phb = PNV_PHB3(dev->parent_bus->parent);
+}
if (!phb) {
error_setg(errp,
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index ffd9d8a947..a0ee52e820 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1782,12 +1782,16 @@ static void pnv_phb4_root_port_realize(DeviceState
*dev, Error **errp)
{
PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev);
PCIDevice *pci = PCI_DEVICE(dev);
-PCIBus *bus = pci_get_bus(pci);
PnvPHB4 *phb = NULL;
Error *local_err = NULL;
-phb = (PnvPHB4 *) object_dynamic_cast(OBJECT(bus->qbus.parent),
- TYPE_PNV_PHB4);
+/*
+ * dev->parent_bus gives access to the pnv-phb-root bus.
+ * The PnvPHB4 is the owner (parent) of the bus.
+ */
+if (dev->parent_bus) {
+phb = PNV_PHB4(dev->parent_bus->parent);
+}
if (!phb) {
error_setg(errp, "%s must be connected to pnv-phb4 buses", dev->id);
--
2.36.1
[PATCH v2 1/9] ppc/pnv: move root port attach to pnv_phb4_realize()
Creating a root port is something related to the PHB, not the PEC. It
also makes the logic more in line with what pnv-phb3 does.
Reviewed-by: Frederic Barrat
Reviewed-by: Cédric Le Goater
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb4.c | 4
hw/pci-host/pnv_phb4_pec.c | 3 ---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 6594016121..23ad8de7ee 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1547,6 +1547,7 @@ static void pnv_phb4_instance_init(Object *obj)
static void pnv_phb4_realize(DeviceState *dev, Error **errp)
{
PnvPHB4 *phb = PNV_PHB4(dev);
+PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
PCIHostState *pci = PCI_HOST_BRIDGE(dev);
XiveSource *xsrc = >xsrc;
int nr_irqs;
@@ -1583,6 +1584,9 @@ static void pnv_phb4_realize(DeviceState *dev, Error
**errp)
pci_setup_iommu(pci->bus, pnv_phb4_dma_iommu, phb);
pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
+/* Add a single Root port if running with defaults */
+pnv_phb_attach_root_port(pci, pecc->rp_model);
+
/* Setup XIVE Source */
if (phb->big_phb) {
nr_irqs = PNV_PHB4_MAX_INTs;
diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
index 8b7e823fa5..c9aaf1c28e 100644
--- a/hw/pci-host/pnv_phb4_pec.c
+++ b/hw/pci-host/pnv_phb4_pec.c
@@ -130,9 +130,6 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState
*pec,
if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
return;
}
-
-/* Add a single Root port if running with defaults */
-pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model);
}
static void pnv_pec_realize(DeviceState *dev, Error **errp)
--
2.36.1
[PATCH v2 2/9] ppc/pnv: attach phb3/phb4 root ports in QOM tree
At this moment we leave the pnv-phb3(4)-root-port unattached in QOM:
/unattached (container)
(...)
/device[2] (pnv-phb3-root-port)
/bus master container[0] (memory-region)
/bus master[0] (memory-region)
/pci_bridge_io[0] (memory-region)
/pci_bridge_io[1] (memory-region)
/pci_bridge_mem[0] (memory-region)
/pci_bridge_pci[0] (memory-region)
/pci_bridge_pref_mem[0] (memory-region)
/pci_bridge_vga_io_hi[0] (memory-region)
/pci_bridge_vga_io_lo[0] (memory-region)
/pci_bridge_vga_mem[0] (memory-region)
/pcie.0 (PCIE)
Let's make changes in pnv_phb_attach_root_port() to attach the created
root ports to its corresponding PHB.
This is the result afterwards:
/pnv-phb3[0] (pnv-phb3)
/lsi (ics)
/msi (phb3-msi)
/msi32[0] (memory-region)
/msi64[0] (memory-region)
/pbcq (pnv-pbcq)
(...)
/phb3_iommu[0] (pnv-phb3-iommu-memory-region)
/pnv-phb3-root.0 (pnv-phb3-root)
/pnv-phb3-root-port[0] (pnv-phb3-root-port)
/bus master container[0] (memory-region)
/bus master[0] (memory-region)
/pci_bridge_io[0] (memory-region)
/pci_bridge_io[1] (memory-region)
/pci_bridge_mem[0] (memory-region)
/pci_bridge_pci[0] (memory-region)
/pci_bridge_pref_mem[0] (memory-region)
/pci_bridge_vga_io_hi[0] (memory-region)
/pci_bridge_vga_io_lo[0] (memory-region)
/pci_bridge_vga_mem[0] (memory-region)
/pcie.0 (PCIE)
Reviewed-by: Frederic Barrat
Reviewed-by: Cédric Le Goater
Signed-off-by: Daniel Henrique Barboza
---
hw/pci-host/pnv_phb3.c | 2 +-
hw/pci-host/pnv_phb4.c | 2 +-
hw/ppc/pnv.c | 7 ++-
include/hw/ppc/pnv.h | 2 +-
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 26ac9b7123..4ba660f8b9 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1052,7 +1052,7 @@ static void pnv_phb3_realize(DeviceState *dev, Error
**errp)
pci_setup_iommu(pci->bus, pnv_phb3_dma_iommu, phb);
-pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), TYPE_PNV_PHB3_ROOT_PORT);
+pnv_phb_attach_root_port(pci, TYPE_PNV_PHB3_ROOT_PORT, phb->phb_id);
}
void pnv_phb3_update_regions(PnvPHB3 *phb)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 23ad8de7ee..ffd9d8a947 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1585,7 +1585,7 @@ static void pnv_phb4_realize(DeviceState *dev, Error
**errp)
pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
/* Add a single Root port if running with defaults */
-pnv_phb_attach_root_port(pci, pecc->rp_model);
+pnv_phb_attach_root_port(pci, pecc->rp_model, phb->phb_id);
/* Setup XIVE Source */
if (phb->big_phb) {
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7c08a78d6c..40e0cbd84d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1190,9 +1190,14 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error
**errp)
}
/* Attach a root port device */
-void pnv_phb_attach_root_port(PCIHostState *pci, const char *name)
+void pnv_phb_attach_root_port(PCIHostState *pci, const char *name, int index)
{
PCIDevice *root = pci_new(PCI_DEVFN(0, 0), name);
+g_autofree char *default_id = g_strdup_printf("%s[%d]", name, index);
+const char *dev_id = DEVICE(root)->id;
+
+object_property_add_child(OBJECT(pci->bus), dev_id ? dev_id : default_id,
+ OBJECT(root));
pci_realize_and_unref(root, pci->bus, _fatal);
}
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 86cb7d7f97..033890a23f 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -189,7 +189,7 @@ DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER10,
TYPE_PNV_CHIP_POWER10)
PowerPCCPU *pnv_chip_find_cpu(PnvChip *chip, uint32_t pir);
-void pnv_phb_attach_root_port(PCIHostState *pci, const char *name);
+void pnv_phb_attach_root_port(PCIHostState *pci, const char *name, int index);
#define TYPE_PNV_MACHINE MACHINE_TYPE_NAME("powernv")
typedef struct PnvMachineClass PnvMachineClass;
--
2.36.1
[PATCH v2 7/9] ppc/pnv: make pnv_chip_power8_pic_print_info() use chip8->phbs[]
It's inneficient to scroll all child objects when we have all PHBs
available in chip8->phbs[].
pnv_chip_power8_pic_print_info_child() ended up folded into
pic_print_info() for simplicity.
Reviewed-by: Frederic Barrat
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/pnv.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 08136def8e..2a9067687b 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -652,25 +652,19 @@ static ISABus *pnv_isa_create(PnvChip *chip, Error **errp)
return PNV_CHIP_GET_CLASS(chip)->isa_create(chip, errp);
}
-static int pnv_chip_power8_pic_print_info_child(Object *child, void *opaque)
-{
-Monitor *mon = opaque;
-PnvPHB3 *phb3 = (PnvPHB3 *) object_dynamic_cast(child, TYPE_PNV_PHB3);
-
-if (phb3) {
-pnv_phb3_msi_pic_print_info(>msis, mon);
-ics_pic_print_info(>lsis, mon);
-}
-return 0;
-}
-
static void pnv_chip_power8_pic_print_info(PnvChip *chip, Monitor *mon)
{
Pnv8Chip *chip8 = PNV8_CHIP(chip);
+int i;
ics_pic_print_info(>psi.ics, mon);
-object_child_foreach(OBJECT(chip),
- pnv_chip_power8_pic_print_info_child, mon);
+
+for (i = 0; i < chip8->num_phbs; i++) {
+PnvPHB3 *phb3 = >phbs[i];
+
+pnv_phb3_msi_pic_print_info(>msis, mon);
+ics_pic_print_info(>lsis, mon);
+}
}
static int pnv_chip_power9_pic_print_info_child(Object *child, void *opaque)
--
2.36.1
[PATCH v2 0/9] pnv-phb related cleanups
This second version contains changes and fixes suggested by Frederic and Cedric in the v1 review. As I've mentioned in the v1, I cropped patches that are more related with the user created device logic. Patches 8-11 will be resend later in another series. I ended up sliding in a couple of new patches (8 and 9) to fix an issue I found while spinning this new version. No functional changes, aside from the QOM change in patch 2, were made intentionally. changes from v2: - patch 3: * removed 'if (dev)' check in pnv_phb3_root_port_realize() - patch 5: * opencoded pnv_ics_get_phb_ics() into pnv_ics_get() * removed pnv_ics_get_phb_ics() * removed ForeachPhb3Args struct - patch 6: * restore the 'ics_resend()' call back in pnv_ics_resend() - patch 8 (new): * remove INTERFACE_PCIE_DEVICE from pnv-phb3-root-bus - patch 9 (new): * remove INTERFACE_PCIE_DEVICE from pnv-phb4-root-bus - v1 link: https://lists.gnu.org/archive/html/qemu-devel/2022-06/msg02333.html Daniel Henrique Barboza (9): ppc/pnv: move root port attach to pnv_phb4_realize() ppc/pnv: attach phb3/phb4 root ports in QOM tree ppc/pnv: use dev->parent_bus->parent to get the PHB ppc/pnv: use dev instead of pci->qdev in root_port_realize() ppc/pnv: make pnv_ics_get() use the chip8->phbs[] array ppc/pnv: make pnv_ics_resend() use chip8->phbs[] ppc/pnv: make pnv_chip_power8_pic_print_info() use chip8->phbs[] ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb3 root bus ppc/pnv: remove 'INTERFACE_PCIE_DEVICE' from phb4 root bus hw/pci-host/pnv_phb3.c | 20 - hw/pci-host/pnv_phb4.c | 23 +- hw/pci-host/pnv_phb4_pec.c | 3 -- hw/ppc/pnv.c | 89 ++ include/hw/ppc/pnv.h | 2 +- 5 files changed, 57 insertions(+), 80 deletions(-) -- 2.36.1
Re: [PATCH qemu v2 2/2] ppc/spapr: Implement H_WATCHDOG
On 6/17/22 08:07, Alexey Kardashevskiy wrote:
The new PAPR 2.12 defines a watchdog facility managed via the new
H_WATCHDOG hypercall.
This adds H_WATCHDOG support which a proposed driver for pseries uses:
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=303120
This was tested by running QEMU with a debug kernel and command line:
-append \
"pseries-wdt.timeout=60 pseries-wdt.nowayout=1 pseries-wdt.action=2"
and running "echo V > /dev/watchdog0" inside the VM.
Signed-off-by: Alexey Kardashevskiy
---
Changes:
v2:
* QOM'ed timers, "action" and "expire" are available via QMP
* removed @timeout from SpaprWatchdog
* moved the driver to hw/watchdog
* fixed error handling in the hcall handler
* used new SETFIELD/GETFIELD
---
include/hw/ppc/spapr.h | 29 +++-
hw/ppc/spapr.c | 4 +
hw/watchdog/spapr_watchdog.c | 248 +++
hw/watchdog/meson.build | 1 +
hw/watchdog/trace-events | 7 +
5 files changed, 288 insertions(+), 1 deletion(-)
create mode 100644 hw/watchdog/spapr_watchdog.c
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 072dda2c7265..ef1e38abd5c7 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -164,6 +164,25 @@ struct SpaprMachineClass {
SpaprIrq *irq;
};
+#define WDT_MAX_WATCHDOGS 4 /* Maximum number of watchdog devices */
+
+#define WDT_HARD_POWER_OFF 0
+#define WDT_HARD_RESTART1
+#define WDT_DUMP_RESTART2
+
+#define TYPE_SPAPR_WDT "spapr-wdt"
+OBJECT_DECLARE_SIMPLE_TYPE(SpaprWatchdog, SPAPR_WDT)
+
+typedef struct SpaprWatchdog {
+/*< private >*/
+DeviceState parent_obj;
+/*< public >*/
+
+unsigned num;
uint8_t should be enough no ? I see num is only used for trace events.
+QEMUTimer timer;
+uint8_t action;
+} SpaprWatchdog;
+
/**
* SpaprMachineState:
*/
@@ -264,6 +283,8 @@ struct SpaprMachineState {
uint32_t FORM2_assoc_array[NUMA_NODES_MAX_NUM][FORM2_NUMA_ASSOC_SIZE];
Error *fwnmi_migration_blocker;
+
+SpaprWatchdog wds[WDT_MAX_WATCHDOGS];
};
#define H_SUCCESS 0
@@ -344,6 +365,7 @@ struct SpaprMachineState {
#define H_P7 -60
#define H_P8 -61
#define H_P9 -62
+#define H_NOOP-63
#define H_UNSUPPORTED -67
#define H_OVERLAP -68
#define H_UNSUPPORTED_FLAG -256
@@ -564,8 +586,9 @@ struct SpaprMachineState {
#define H_SCM_HEALTH0x400
#define H_RPT_INVALIDATE0x448
#define H_SCM_FLUSH 0x44C
+#define H_WATCHDOG 0x45C
-#define MAX_HCALL_OPCODEH_SCM_FLUSH
+#define MAX_HCALL_OPCODEH_WATCHDOG
/* The hcalls above are standardized in PAPR and implemented by pHyp
* as well.
@@ -1027,6 +1050,7 @@ extern const VMStateDescription
vmstate_spapr_cap_large_decr;
extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
extern const VMStateDescription vmstate_spapr_cap_fwnmi;
extern const VMStateDescription vmstate_spapr_cap_rpt_invalidate;
+extern const VMStateDescription vmstate_spapr_wdt;
static inline uint8_t spapr_get_cap(SpaprMachineState *spapr, int cap)
{
@@ -1063,4 +1087,7 @@ target_ulong
spapr_vof_client_architecture_support(MachineState *ms,
target_ulong ovec_addr);
void spapr_vof_client_dt_finalize(SpaprMachineState *spapr, void *fdt);
+/* H_WATCHDOG */
+void spapr_watchdog_init(SpaprMachineState *spapr);
+
#endif /* HW_SPAPR_H */
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fd4942e8813c..9a5382d5270f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -898,6 +898,8 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void
*fdt)
add_str(hypertas, "hcall-hpt-resize");
}
+add_str(hypertas, "hcall-watchdog");
+
_FDT(fdt_setprop(fdt, rtas, "ibm,hypertas-functions",
hypertas->str, hypertas->len));
g_string_free(hypertas, TRUE);
@@ -3051,6 +3053,8 @@ static void spapr_machine_init(MachineState *machine)
spapr->vof->fw_size = fw_size; /* for claim() on itself */
spapr_register_hypercall(KVMPPC_H_VOF_CLIENT, spapr_h_vof_client);
}
+
+spapr_watchdog_init(spapr);
}
#define DEFAULT_KVM_TYPE "auto"
diff --git a/hw/watchdog/spapr_watchdog.c b/hw/watchdog/spapr_watchdog.c
new file mode 100644
index ..aeaf7c52cbad
--- /dev/null
+++ b/hw/watchdog/spapr_watchdog.c
@@ -0,0 +1,248 @@
+/*
+ * 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.1 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
Re: [PATCH qemu v2 1/2] ppc: Define SETFIELD for the ppc target
On 6/17/22 08:07, Alexey Kardashevskiy wrote:
It keeps repeating, move it to the header. This uses __builtin_ctzl() to
allow using the macros in #define.
Signed-off-by: Alexey Kardashevskiy
Reviewed-by: Cédric Le Goater
Thanks Alexey,
C.
---
include/hw/pci-host/pnv_phb3_regs.h | 16
target/ppc/cpu.h| 5 +
hw/intc/pnv_xive.c | 20
hw/intc/pnv_xive2.c | 20
hw/pci-host/pnv_phb4.c | 16
5 files changed, 5 insertions(+), 72 deletions(-)
diff --git a/include/hw/pci-host/pnv_phb3_regs.h
b/include/hw/pci-host/pnv_phb3_regs.h
index a174ef1f7045..38f8ce9d7406 100644
--- a/include/hw/pci-host/pnv_phb3_regs.h
+++ b/include/hw/pci-host/pnv_phb3_regs.h
@@ -12,22 +12,6 @@
#include "qemu/host-utils.h"
-/*
- * QEMU version of the GETFIELD/SETFIELD macros
- *
- * These are common with the PnvXive model.
- */
-static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
-{
-return (word & mask) >> ctz64(mask);
-}
-
-static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
-uint64_t value)
-{
-return (word & ~mask) | ((value << ctz64(mask)) & mask);
-}
-
/*
* PBCQ XSCOM registers
*/
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 6d78078f379d..9a1f1ea3 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -47,6 +47,11 @@
PPC_BIT32(bs))
#define PPC_BITMASK8(bs, be)((PPC_BIT8(bs) - PPC_BIT8(be)) | PPC_BIT8(bs))
+#define GETFIELD(mask, word) \
+(((word) & (mask)) >> __builtin_ctzl(mask))
+#define SETFIELD(mask, word, val) \
+(((word) & ~(mask)) | (((uint64_t)(val) << __builtin_ctzl(mask)) & (mask)))
+
/*/
/* Exception vectors definitions
*/
enum {
diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 1ce1d7b07d63..c7b75ed12ee0 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -66,26 +66,6 @@ static const XiveVstInfo vst_infos[] = {
qemu_log_mask(LOG_GUEST_ERROR, "XIVE[%x] - " fmt "\n", \
(xive)->chip->chip_id, ## __VA_ARGS__);
-/*
- * QEMU version of the GETFIELD/SETFIELD macros
- *
- * TODO: It might be better to use the existing extract64() and
- * deposit64() but this means that all the register definitions will
- * change and become incompatible with the ones found in skiboot.
- *
- * Keep it as it is for now until we find a common ground.
- */
-static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
-{
-return (word & mask) >> ctz64(mask);
-}
-
-static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
-uint64_t value)
-{
-return (word & ~mask) | ((value << ctz64(mask)) & mask);
-}
-
/*
* When PC_TCTXT_CHIPID_OVERRIDE is configured, the PC_TCTXT_CHIPID
* field overrides the hardwired chip ID in the Powerbus operations
diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
index a39e070e82d2..3fe349749384 100644
--- a/hw/intc/pnv_xive2.c
+++ b/hw/intc/pnv_xive2.c
@@ -75,26 +75,6 @@ static const XiveVstInfo vst_infos[] = {
qemu_log_mask(LOG_GUEST_ERROR, "XIVE[%x] - " fmt "\n", \
(xive)->chip->chip_id, ## __VA_ARGS__);
-/*
- * QEMU version of the GETFIELD/SETFIELD macros
- *
- * TODO: It might be better to use the existing extract64() and
- * deposit64() but this means that all the register definitions will
- * change and become incompatible with the ones found in skiboot.
- *
- * Keep it as it is for now until we find a common ground.
- */
-static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
-{
-return (word & mask) >> ctz64(mask);
-}
-
-static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
-uint64_t value)
-{
-return (word & ~mask) | ((value << ctz64(mask)) & mask);
-}
-
/*
* TODO: Document block id override
*/
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 13ba9e45d8b6..0913e7c8f015 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -31,22 +31,6 @@
qemu_log_mask(LOG_GUEST_ERROR, "phb4_pec[%d:%d]: " fmt "\n",\
(pec)->chip_id, (pec)->index, ## __VA_ARGS__)
-/*
- * QEMU version of the GETFIELD/SETFIELD macros
- *
- * These are common with the PnvXive model.
- */
-static inline uint64_t GETFIELD(uint64_t mask, uint64_t word)
-{
-return (word & mask) >> ctz64(mask);
-}
-
-static inline uint64_t SETFIELD(uint64_t mask, uint64_t word,
-uint64_t value)
-{
-return (word & ~mask) | ((value << ctz64(mask)) & mask);
-}
-
static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB4 *phb)
{
PCIHostState *pci = PCI_HOST_BRIDGE(phb);
Re: [PATCH 4/4] slirp: Add oob-eth-addr to -netdev options
Peter Delevoryas, le mer. 15 juin 2022 18:05:26 -0700, a ecrit:
> With this change, you can now request the out-of-band MAC address from
> slirp in fby35-bmc:
>
> wget
> https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> qemu-system-arm -machine fby35-bmc \
> -drive file=fby35.mtd,format=raw,if=mtd \
> -nographic \
> -netdev
> user,id=nic,mfr-id=0x8119,oob-eth-addr=de:ad:be:ef:ca:fe,hostfwd=::-:22 \
> -net nic,model=ftgmac100,netdev=nic
>
> ...
> username: root
> password: 0penBmc
> ...
>
> root@bmc-oob:~# ncsi-util -n eth0 -c 0 0x50 0 0 0x81 0x19 0 0 0x1b 0
> NC-SI Command Response:
> cmd: NCSI_OEM_CMD(0x50)
> Response: COMMAND_COMPLETED(0x) Reason: NO_ERROR(0x)
> Payload length = 24
>
> 20: 0x00 0x00 0x81 0x19
> 24: 0x01 0x00 0x1b 0x00
> 28: 0x00 0x00 0x00 0x00
> 32: 0xde 0xad 0xbe 0xef
> 36: 0xca 0xfe 0x00 0x00
>
> root@bmc-oob:~# ifconfig
> eth0 Link encap:Ethernet HWaddr DE:AD:BE:EF:CA:FE
> inet addr:10.0.2.15 Bcast:10.0.2.255 Mask:255.255.255.0
> inet6 addr: fec0::dcad:beff:feef:cafe/64 Scope:Site
> inet6 addr: fe80::dcad:beff:feef:cafe/64 Scope:Link
> UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
> RX packets:253 errors:0 dropped:0 overruns:0 frame:0
> TX packets:271 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:1000
> RX bytes:24638 (24.0 KiB) TX bytes:18876 (18.4 KiB)
> Interrupt:32
>
> loLink encap:Local Loopback
> inet addr:127.0.0.1 Mask:255.0.0.0
> inet6 addr: ::1/128 Scope:Host
> UP LOOPBACK RUNNING MTU:65536 Metric:1
> RX packets:2 errors:0 dropped:0 overruns:0 frame:0
> TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
> collisions:0 txqueuelen:1000
> RX bytes:120 (120.0 B) TX bytes:120 (120.0 B)
>
> Signed-off-by: Peter Delevoryas
> ---
> net/slirp.c | 13 +++--
> qapi/net.json | 5 -
> 2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 231068c1e2..858d3da859 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -414,7 +414,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
>const char *tftp_server_name, uint32_t mfr_id,
> - Error **errp)
> + uint8_t oob_eth_addr[ETH_ALEN], Error **errp)
> {
> /* default settings according to historic slirp */
> struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */
> @@ -637,6 +637,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> cfg.vdnssearch = dnssearch;
> cfg.vdomainname = vdomainname;
> cfg.mfr_id = mfr_id;
> +memcpy(cfg.oob_eth_addr, oob_eth_addr, ETH_ALEN);
And similarly here.
> s->slirp = slirp_new(, _cb, s);
> QTAILQ_INSERT_TAIL(_stacks, s, entry);
>
> @@ -1142,6 +1143,7 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> const NetdevUserOptions *user;
> const char **dnssearch;
> bool ipv4 = true, ipv6 = true;
> +MACAddr oob_eth_addr = {};
>
> assert(netdev->type == NET_CLIENT_DRIVER_USER);
> user = >u.user;
> @@ -1166,6 +1168,12 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
> net_init_slirp_configs(user->guestfwd, 0);
>
> +if (user->has_oob_eth_addr &&
> +net_parse_macaddr(oob_eth_addr.a, user->oob_eth_addr) < 0) {
> +error_setg(errp, "invalid syntax for OOB ethernet address");
> +return -1;
> +}
> +
> ret = net_slirp_init(peer, "user", name, user->q_restrict,
> ipv4, vnet, user->host,
> ipv6, user->ipv6_prefix, user->ipv6_prefixlen,
> @@ -1173,7 +1181,8 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> user->bootfile, user->dhcpstart,
> user->dns, user->ipv6_dns, user->smb,
> user->smbserver, dnssearch, user->domainname,
> - user->tftp_server_name, user->mfr_id, errp);
> + user->tftp_server_name, user->mfr_id,
> oob_eth_addr.a,
> + errp);
>
> while (slirp_configs) {
> config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index efc5cb3fb6..7b2c3c205c 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -169,6 +169,8 @@
> #
> # @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> #
> +# @oob-eth-addr:
Re: [PATCH 3/4] slirp: Add mfr-id to -netdev options
Peter Delevoryas, le mer. 15 juin 2022 18:05:25 -0700, a ecrit:
> This lets you set the manufacturer's ID for a slirp netdev, which can be
> queried from the guest through the Get Version ID NC-SI command. For
> example, by setting the manufacturer's ID to 0x8119:
>
> wget
> https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> qemu-system-arm -machine fby35-bmc \
> -drive file=fby35.mtd,format=raw,if=mtd -nographic \
> -netdev user,id=nic,mfr-id=0x8119,hostfwd=::-:22 \
> -net nic,model=ftgmac100,netdev=nic
> ...
> username: root
> password: 0penBmc
> ...
> root@bmc-oob:~# ncsi-util 0x15
> NC-SI Command Response:
> cmd: GET_VERSION_ID(0x15)
> Response: COMMAND_COMPLETED(0x) Reason: NO_ERROR(0x)
> Payload length = 40
>
> 20: 0xf1 0xf0 0xf0 0x00
> 24: 0x00 0x00 0x00 0x00
> 28: 0x00 0x00 0x00 0x00
> 32: 0x00 0x00 0x00 0x00
> 36: 0x00 0x00 0x00 0x00
> 40: 0x00 0x00 0x00 0x00
> 44: 0x00 0x00 0x00 0x00
> 48: 0x00 0x00 0x00 0x00
> 52: 0x00 0x00 0x81 0x19
>
> Signed-off-by: Peter Delevoryas
> ---
> net/slirp.c | 5 +++--
> qapi/net.json | 5 -
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index 75e5ccafd9..231068c1e2 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -413,7 +413,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
>const char *vnameserver, const char *vnameserver6,
>const char *smb_export, const char *vsmbserver,
>const char **dnssearch, const char *vdomainname,
> - const char *tftp_server_name,
> + const char *tftp_server_name, uint32_t mfr_id,
>Error **errp)
> {
> /* default settings according to historic slirp */
> @@ -636,6 +636,7 @@ static int net_slirp_init(NetClientState *peer, const
> char *model,
> cfg.vnameserver6 = ip6_dns;
> cfg.vdnssearch = dnssearch;
> cfg.vdomainname = vdomainname;
> +cfg.mfr_id = mfr_id;
You will need a #if to only include it with slirp 4.8.0 indeed.
Otherwise the mfr_id field won't exist.
In the #else part, it would probably useful to give the user at least a
warning that tells her to upgrade slirp to 4.8.0 to get the mfr_id
functionality working.
> s->slirp = slirp_new(, _cb, s);
> QTAILQ_INSERT_TAIL(_stacks, s, entry);
>
> @@ -1172,7 +1173,7 @@ int net_init_slirp(const Netdev *netdev, const char
> *name,
> user->bootfile, user->dhcpstart,
> user->dns, user->ipv6_dns, user->smb,
> user->smbserver, dnssearch, user->domainname,
> - user->tftp_server_name, errp);
> + user->tftp_server_name, user->mfr_id, errp);
>
> while (slirp_configs) {
> config = slirp_configs;
> diff --git a/qapi/net.json b/qapi/net.json
> index d6f7cfd4d6..efc5cb3fb6 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -167,6 +167,8 @@
> #
> # @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
> #
> +# @mfr-id: Manufacturer ID (Private Enterprise Number: IANA)
> +#
> # Since: 1.2
> ##
> { 'struct': 'NetdevUserOptions',
> @@ -192,7 +194,8 @@
> '*smbserver': 'str',
> '*hostfwd': ['String'],
> '*guestfwd': ['String'],
> -'*tftp-server-name': 'str' } }
> +'*tftp-server-name': 'str',
> +'*mfr-id': 'uint32' } }
>
> ##
> # @NetdevTapOptions:
> --
> 2.30.2
>
Re: [PATCH 2/4] slirp: Update SlirpConfig version to 5
Hello, Peter Delevoryas, le mer. 15 juin 2022 18:05:24 -0700, a ecrit: > I think we probably need a new Slirp release > (4.8.0) and a switch statement here instead, right? > > So that we can preserve the behavior for 4.7.0? Yes, that's the idea. Samuel
[PATCH] aspeed/hace: Add missing newlines to unimp messages
Signed-off-by: Joel Stanley
---
hw/misc/aspeed_hace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 4b5997e18fad..731234b78c4c 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -340,12 +340,12 @@ static void aspeed_hace_write(void *opaque, hwaddr addr,
uint64_t data,
if ((data & HASH_HMAC_MASK)) {
qemu_log_mask(LOG_UNIMP,
- "%s: HMAC engine command mode %"PRIx64" not
implemented",
+ "%s: HMAC engine command mode %"PRIx64" not
implemented\n",
__func__, (data & HASH_HMAC_MASK) >> 8);
}
if (data & BIT(1)) {
qemu_log_mask(LOG_UNIMP,
- "%s: Cascaded mode not implemented",
+ "%s: Cascaded mode not implemented\n",
__func__);
}
algo = hash_algo_lookup(data);
--
2.35.1
