Re: ebpf/rss.bpf.skeleton.h generated by what?
On 5/7/22 08:53, Jason Wang wrote: On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini wrote: On 5/7/22 07:05, Jason Wang wrote: If it was generated by something in the tree, it should not be committed to git. Doesn't look like it is. Andrew may know more. I remember it was generated by libbpf. What is the source? It's tools/ebpf/rss.bpf.c. Thanks, maybe we can reuse some of the support for cross compilation that will be introduced soon. Paolo
Re: ebpf/rss.bpf.skeleton.h generated by what?
On Sat, May 7, 2022 at 2:58 PM Paolo Bonzini wrote: > > On 5/7/22 08:53, Jason Wang wrote: > > On Sat, May 7, 2022 at 2:46 PM Paolo Bonzini wrote: > >> > >> On 5/7/22 07:05, Jason Wang wrote: > If it was generated by something in the tree, it should not be committed > to git. Doesn't look like it is. > >>> Andrew may know more. > >>> > >>> I remember it was generated by libbpf. > >> > >> What is the source? > > > > It's tools/ebpf/rss.bpf.c. > > Thanks, maybe we can reuse some of the support for cross compilation > that will be introduced soon. That would be better. The reason we don't auto generate during make is to avoid adding new dependencies (where libbpf/bpftool is not popular at that time). Thanks > > Paolo >
[PATCH 0/2] hw/nvme: support smart AEN
Hi, In this series, firstly introduce smart related bits of aen cfg, then support this in oaes. Linux guest does not support this currently, I also send a series to enable smart AEN: https://lore.kernel.org/lkml/20220507065026.260306-1-pizhen...@bytedance.com/T/#t Test the two series together, works fine. Zhenwei Pi (2): hw/nvme: introduce smart bits of aen cfg hw/nvme: support smart AEN hw/nvme/ctrl.c | 9 - include/block/nvme.h | 8 +++- 2 files changed, 15 insertions(+), 2 deletions(-) -- 2.20.1
[PATCH 1/2] hw/nvme: introduce smart bits of aen cfg
According to NVM Express v1.4, Section 5.21.1.11 (Asynchronous Event Configuration), introduce bit 0 ~ bit 5. Signed-off-by: zhenwei pi --- include/block/nvme.h | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/include/block/nvme.h b/include/block/nvme.h index 3737351cc8..d92912f9ad 100644 --- a/include/block/nvme.h +++ b/include/block/nvme.h @@ -1122,7 +1122,13 @@ typedef struct NvmeIdCtrlNvm { } NvmeIdCtrlNvm; enum NvmeIdCtrlOaes { -NVME_OAES_NS_ATTR = 1 << 8, +NVME_OAES_SMART_SPARE = NVME_SMART_SPARE, +NVME_OAES_SMART_TEMPERATURE = NVME_SMART_TEMPERATURE, +NVME_OAES_SMART_RELIABILITY = NVME_SMART_RELIABILITY, +NVME_OAES_SMART_MEDIA_READ_ONLY = NVME_SMART_MEDIA_READ_ONLY, +NVME_OAES_SMART_FAILED_VOLATILE_MEDIA = NVME_SMART_FAILED_VOLATILE_MEDIA, +NVME_OAES_SMART_PMR_UNRELIABLE= NVME_SMART_PMR_UNRELIABLE, +NVME_OAES_NS_ATTR = 1 << 8, }; enum NvmeIdCtrlCtratt { -- 2.20.1
[PATCH 2/2] hw/nvme: support smart AEN
Support smart AEN on controller side, if the guest side enables this feature, after injecting smart critical warning, also raise AER. This can be tested by: virsh qemu-monitor-command vm '{ "execute": "qom-set", "arguments": { "path": "/machine/peripheral/nvme0", "property": "smart_critical_warning", "value":1 } }' Signed-off-by: zhenwei pi --- hw/nvme/ctrl.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 03760ddeae..8236a746c8 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -6707,6 +6707,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) NvmeIdCtrl *id = &n->id_ctrl; uint8_t *pci_conf = pci_dev->config; uint64_t cap = ldq_le_p(&n->bar.cap); +uint32_t supported_oaes; id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID)); id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID)); @@ -6716,7 +6717,13 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev) id->cntlid = cpu_to_le16(n->cntlid); -id->oaes = cpu_to_le32(NVME_OAES_NS_ATTR); +supported_oaes = NVME_OAES_SMART_SPARE | NVME_OAES_SMART_TEMPERATURE | + NVME_OAES_SMART_RELIABILITY | + NVME_OAES_SMART_MEDIA_READ_ONLY | + NVME_OAES_SMART_FAILED_VOLATILE_MEDIA | + NVME_OAES_SMART_PMR_UNRELIABLE | + NVME_OAES_NS_ATTR; +id->oaes = cpu_to_le32(supported_oaes); id->ctratt |= cpu_to_le32(NVME_CTRATT_ELBAS); id->rab = 6; -- 2.20.1
Re: [PATCH 4/5] hw/intc/arm_gicv3: Use correct number of priority bits for the CPU
Peter, I’ll talk with Shuichiro this coming Monday (here most of us on vacation), and get back to you. Itaru. On Sat, May 7, 2022 at 1:34 Peter Maydell wrote: > On Fri, 6 May 2022 at 17:21, Peter Maydell > wrote: > > > > Make the GICv3 set its number of bits of physical priority from the > > implementation-specific value provided in the CPU state struct, in > > the same way we already do for virtual priority bits. Because this > > would be a migration compatibility break, we provide a property > > force-8-bit-prio which is enabled for 7.0 and earlier versioned board > > models to retain the legacy "always use 8 bits" behaviour. > > > > Signed-off-by: Peter Maydell > > --- > > I have guessed at the right value for the A64FX, but if we can > > find the correct ICC_CTLR_EL1 value that would be better. > > Shuuichirou, Itaru: do either of you know the right setting for > the A64FX for this? If you can find what the hardware value of > the ICC_CTLR_EL3 or ICC_CTLR_EL1 register is (more specifically, > the PRIBits subfield) that should be enough to tell us. > > > @@ -961,6 +964,12 @@ static void aarch64_a64fx_initfn(Object *obj) > > cpu->gic_num_lrs = 4; > > cpu->gic_vpribits = 5; > > cpu->gic_vprebits = 5; > > +/* > > + * TODO: What does the real A64FX GICv3 provide ? > > + * This is a guess based on what other Arm CPUs do; to find the > correct > > + * answer we need the value of the A64FX's ICC_CTLR_EL1 register. > > + */ > > +cpu->gic_pribits = 5; > > > > /* Suppport of A64FX's vector length are 128,256 and 512bit only */ > > aarch64_add_sve_properties(obj); > > -- > > 2.25.1 > > thanks > -- PMM >
Re: [PULL v2 00/18] target/xtensa updates for v7.1
On 5/6/22 17:43, Max Filippov wrote: Hello, please pull the following updates for the target/xtensa. Changes since v1: - rebase series to the current master - drop big-endian tests enabling patch (cannot test it because of the test infrastructure change) - add cache testing opcodes patch The following changes since commit 31abf61c4929a91275fe32f1fafe6e6b3e840b2a: Merge tag 'pull-ppc-20220505' of https://gitlab.com/danielhb/qemu into staging (2022-05-05 13:52:22 -0500) are available in the Git repository at: https://github.com/OSLL/qemu-xtensa.git tags/20220506-xtensa-1 for you to fetch changes up to 59491e97f89eaeee275f57fb6bb40f0152429fb3: target/xtensa: implement cache test option opcodes (2022-05-06 15:37:10 -0700) target/xtensa updates for v7.1: - expand test coverage to MMUv3, cores without windowed registers or loop option; - import lx106 core (used in the esp8266 IoT chips); - use tcg_constant_* in the front end; - add clock input to the xtensa CPU; - fix reset state of the xtensa MX PIC; - implement cache testing opcodes. Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Max Filippov (17): target/xtensa: fix missing tcg_temp_free in gen_window_check target/xtensa: use tcg_contatnt_* for numeric literals target/xtensa: use tcg_constant_* for exceptions target/xtensa: use tcg_constant_* for TLB opcodes target/xtensa: use tcg_constant_* for numbered special registers target/xtensa: use tcg_constant_* for FPU conversion opcodes target/xtensa: use tcg_constant_* for remaining opcodes target/xtensa: add clock input to xtensa CPU hw/xtensa: fix reset value of MIROUT register of MX PIC tests/tcg/xtensa: fix build for cores without windowed registers tests/tcg/xtensa: restore vecbase SR after test tests/tcg/xtensa: fix watchpoint test tests/tcg/xtensa: remove dependency on the loop option tests/tcg/xtensa: enable autorefill phys_mem tests for MMUv3 tests/tcg/xtensa: enable mmu tests for MMUv3 tests/tcg/xtensa: fix vectors and checks in timer test target/xtensa: implement cache test option opcodes Simon Safar (1): target/xtensa: import core lx106 hw/xtensa/mx_pic.c|2 +- target/xtensa/core-lx106.c| 51 + target/xtensa/core-lx106/core-isa.h | 470 ++ target/xtensa/core-lx106/gdb-config.c.inc | 83 + target/xtensa/core-lx106/xtensa-modules.c.inc | 7668 + target/xtensa/cores.list |1 + target/xtensa/cpu.c | 15 + target/xtensa/cpu.h |5 + target/xtensa/op_helper.c |7 +- target/xtensa/translate.c | 211 +- tests/tcg/xtensa/crt.S|2 + tests/tcg/xtensa/test_break.S | 86 +- tests/tcg/xtensa/test_mmu.S | 182 +- tests/tcg/xtensa/test_phys_mem.S | 10 +- tests/tcg/xtensa/test_sr.S|2 + tests/tcg/xtensa/test_timer.S | 68 +- 16 files changed, 8604 insertions(+), 259 deletions(-) create mode 100644 target/xtensa/core-lx106.c create mode 100644 target/xtensa/core-lx106/core-isa.h create mode 100644 target/xtensa/core-lx106/gdb-config.c.inc create mode 100644 target/xtensa/core-lx106/xtensa-modules.c.inc
Re: [PATCH 0/4] Clean up header guards again
On 5/6/22 08:49, Markus Armbruster wrote: Markus Armbruster (4): Clean up header guards that don't match their file name Clean up ill-advised or unusual header guards Normalize header guard symbol definition Clean up decorations and whitespace around header guards Reviewed-by: Richard Henderson r~
Re: [PATCH 0/5] gicv3: Use right number of prio bits for the CPU
On 5/6/22 11:21, Peter Maydell wrote: This patchset fills in an odd inconsistency in our GICv3 emulation that I noticed while I was doing the GICv4 work. At the moment we allow the CPU to specify the number of bits of virtual priority (via the ARMCPU::gic_vpribits field), but we always use 8 bits of physical priority, even though to my knowledge no real Arm CPU hardware has that many. This series makes the GICv3 emulation use a runtime-configurable number of physical priority bits, and sets it to match the number used by the various CPUs we implement (which is 5 for all the Cortex-Axx CPUs we emulate). Because changing the number of priority bits is a migration compatibility break, we use a compat property to keep the number of priority bits at 8 for older versions of the virt board. There is one TODO left in this series, which is that I don't know the right value to use for the A64FX, so I've guessed that it is 5, like all the Arm implementations. Patch 1 is an independent bugfix; patch 5 is cleanup. thanks -- PMM Peter Maydell (5): hw/intc/arm_gicv3: report correct PRIbits field in ICV_CTLR_EL1 hw/intc/arm_gicv3_kvm.c: Stop using GIC_MIN_BPR constant hw/intc/arm_gicv3: Support configurable number of physical priority bits hw/intc/arm_gicv3: Use correct number of priority bits for the CPU hw/intc/arm_gicv3: Provide ich_num_aprs() Reviewed-by: Richard Henderson r~
Re: [PATCH 5/5] hw/intc/arm_gicv3: Provide ich_num_aprs()
On 5/6/22 11:21, Peter Maydell wrote: @@ -145,11 +153,8 @@ static int ich_highest_active_virt_prio(GICv3CPUState *cs) * in the ICH Active Priority Registers. */ int i; -int aprmax = 1 << (cs->vprebits - 5); -assert(aprmax <= ARRAY_SIZE(cs->ich_apr[0])); - -for (i = 0; i < aprmax; i++) { +for (i = 0; i < ich_num_aprs(cs); i++) { Better to retain the local variable for the loop bound. r~
Re: [PULL v5 00/25] Misc patches for 2022-04-29
On 5/7/22 00:50, Paolo Bonzini wrote: The following changes since commit 13220a46e27ef95159651acd5e408b6aac9dbf3e: Merge tag 'vfio-updates-20220506.1' of https://gitlab.com/alex.williamson/qemu into staging (2022-05-06 16:18:14 -0500) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 6033b9ecd4f6a26b78f44a94813e1e464f5b0549: pc: remove -soundhw pcspk (2022-05-07 07:46:59 +0200) * WHPX support for xcr0 * qga-wss fixes * Meson conversions * Removed -soundhw pcspk Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Konstantin Kostiuk (2): configure: Add cross prefix for widl tool qga-vss: always build qga-vss.tlb when qga-vss.dll is built Paolo Bonzini (22): meson-buildoptions: add support for string options meson, configure: move Xen detection to meson configure, meson: move iasl detection to meson configure: move Windows flags detection to meson configure: switch string options to automatic parsing meson, configure: move --tls-priority to meson meson, configure: move bdrv whitelists to meson meson, configure: move --with-pkgversion, CONFIG_STAMP to meson meson, configure: move --interp-prefix to meson meson: always combine directories with prefix configure: switch directory options to automatic parsing meson: pass more options directly as -D configure: omit options with default values from meson command line meson, virtio: place all virtio-pci devices under virtio_pci_ss configure: simplify vhost-net-{user, vdpa} configuration build: move vhost-vsock configuration to Kconfig build: move vhost-scsi configuration to Kconfig build: move vhost-user-fs configuration to Kconfig meson: create have_vhost_* variables meson: use have_vhost_* variables to pick sources configure, meson: move vhost options to Meson pc: remove -soundhw pcspk Sunil Muthuswamy (1): WHPX: support for xcr0 Kconfig.host| 3 - backends/meson.build| 8 +- configure | 673 ++-- docs/meson.build| 2 +- hw/audio/pcspk.c| 10 - hw/audio/soundhw.c | 27 +- hw/net/meson.build | 8 +- hw/scsi/Kconfig | 5 + hw/virtio/Kconfig | 18 +- hw/virtio/meson.build | 34 +- include/hw/audio/soundhw.h | 3 - include/hw/virtio/virtio-scsi.h | 2 - meson.build | 256 ++--- meson_options.txt | 28 +- net/meson.build | 12 +- qga/vss-win32/meson.build | 4 +- scripts/ci/org.centos/stream/8/x86_64/configure | 3 - scripts/meson-buildoptions.py | 86 ++- scripts/meson-buildoptions.sh | 74 ++- scripts/qemu-stamp.py | 24 + scripts/xen-detect.c| 203 +++ target/i386/whpx/whpx-all.c | 87 +++ target/i386/whpx/whpx-internal.h| 3 + tests/meson.build | 2 +- tests/qtest/meson.build | 4 +- tools/meson.build | 2 +- 26 files changed, 776 insertions(+), 805 deletions(-) create mode 100644 scripts/qemu-stamp.py create mode 100644 scripts/xen-detect.c
Possible bug in Aarch64 single-stepping
Hi, I’m writing a simple debugger in assembly code for the Raspberry Pi 3B (in aarch64). I’m using QEMU 7.0.0. Everything is running in EL1. (I have MDE and KDE set in MDSCR_EL1). I’m coming across Unexpected Behaviour when playing with single-stepping: It appears that single-stepping is enabled (ie. an exception is generated after every instruction) when the SS bit (bit-0) is set in MDSCR_EL1 and debug events are enabled in the CPSR (“D” bit clear) *** irrespective of whether the SS bit (bit-21) is set in CPSR or not ***. I thought the SS bit (bit-21) needs to be set in CPSR for single-stepping to occur (and that it gets cleared whenever an exception is taken and needs to be reset if one wants to single-step again). Have I misunderstood / misconfigured something, or is this a bug? Attached is a minimal(ish) example: Regards, chris /* ** built with aarch64-elf-as -march=armv8-a+crc+crypto -mcpu=cortex-a53 --gstabs minimal.s -o minimal.o aarch64-elf-ld minimal.o -T memmap -o minimal.elf aarch64-elf-objcopy -O binary minimal.elf minimal.img aarch64-elf-objdump -d -j .text minimal.elf > minimal.list memmap is MEMORY { ram : ORIGIN = 0x8, LENGTH = 0x8000 } SECTIONS { .init : { *(.init*) } > ram .text : { *(.text*) } > ram .data : { *(.data*) } > ram .bss : { *(.bss*) } > ram } qemu cmd qemu-7.0.0/build/qemu-system-aarch64 -M raspi3b -kernel minimal.img -serial null -serial stdio -s -S gdb cmd /opt/local/gnuaarch64/bin/aarch64-none-elf-gdb -x gdb.cmds minimal.elf gdb.cmds is target remote :1234 set $pc = 0x8 ** */ .section .text .global _start _start: mrs x0, mpidr_el1 // x0 = .8000. and x0, x0, 0b11 cbz x0, 2f // branch fwd if core == 0 ... 1: wfe // ... else wait for exception (ie sleep) b 1b 2: // mrs x0, scr_el3 // Can't access this from EL2. But gdb says it's 0x0501 // .0101|.0001 10:RW=1 8:HCE=1 0:NS=1 // mrs x0, hcr_el2 // x0 -> 1<<31 (34:E2H=0 31:RW=1 27:TGE=0 2:C=0 1:M=0) // mrs x0, sctlr_el1 // x0 -> 00c5.0838 = .|1100.0101|.1000|0011.1000 mov x0, 0x0800 // -SA -SA0, -CP15BEN -nTWE -nTWI +20:RES1 +28:RES1 +29:RES1 movkx0, 0x30d0, lsl 16 // x0 -> 30d0.0800 = 0011.|1101.|.1000|. msr sctlr_el1, x0 adr x0, vectors msr vbar_el1, x0 adr x0, _start // 0x0008. = 512k msr sp_el1, x0 // set EL1's SP mov x0, 0x3c5 // NOTE: bit 21:SS is NOT set msr spsr_el2, x0// DAIF exceptions masked. A64 state. EL1. SP = SP_ELx adr x0, init msr elr_el2, x0 eret// We're in EL2. Pretend we're "returning" to EL1 init: init: msr OSLAR_EL1, xzr // unlock OS lock mov x0, 0b101<<13 // 15:MDE (enable debugging) and 13:KDE (and kernel debug events) orr x0, x0, 0b1<<0 // enable SS debug exceptions msr MDSCR_EL1, x0 mov w0, 0 // these all execute ok mov w0, 1 b 1f mov w0, -1 1: msr daifclr, 0b // enable all interrupts and debugging exceptions mov w0, 3 // this causes an exception ESR_EL1 = 0xce22 (see ARMv8ARM D1-1801) mov w0, 4 // 0xce = 0b1100.1110|- >> 26 = 0b0011.0011 = 0x33 = exception class // ie. SS from same EL. ** even though CPSR was 0x0005 ** svc 124 mov w0, 5 b 2f mov w0, -2 2: mov w0, 6 loop: b loop // ** exc_svc: and w0, w0, 0x cmp w0, 123 beq svc_123 cmp w0, 124 beq svc_124 b exception_return// undefined SVCs ignored svc_123: mrs x0, SPSR_EL1 orr x0, x0, 0b1<<21 // (re)enable SS in CPSR msr SPSR_EL1, x0 b exception_return svc_124: mrs x0, SPSR_EL1 bic x0, x0, 0b1<<21 // disable SS in CPSR (not really necessary since SS gets msr SPSR_EL1, x0// cleared when an exception (eg. SVC) gets taken anyway) b exception_return exc_dbg: mrs x0, elr_el1 mrs x0, SPSR_EL1 bic x0, x0, 0b1<<21 // (re)enable SS in CPSR
[PATCH] cpu-throttle: reset throttle_thread_scheduled when throttle_percentage is 0
From: yilingjin The throttle_thread_scheduled flag is set to 1 in cpu_throttle_timer_tick() when throttle_percentage isn't 0, and will reset back to 0 after sleeping in cpu_throttle_thread(). Given that throttle_timer may tick with a slight delay, the throttle_percentage may reset to 0 before schedule cpu_throttle_thread(). This results on cpu_throttle_thread() to return immediately without reset throttle_thread_scheduled flag back to 0, so there is no longer any chances for vCPU thread to sleep in cpu_throttle_thread(). Signed-off-by: yilingjin --- softmmu/cpu-throttle.c | 1 + 1 file changed, 1 insertion(+) diff --git a/softmmu/cpu-throttle.c b/softmmu/cpu-throttle.c index d9bb30a223..7c2fd26072 100644 --- a/softmmu/cpu-throttle.c +++ b/softmmu/cpu-throttle.c @@ -44,6 +44,7 @@ static void cpu_throttle_thread(CPUState *cpu, run_on_cpu_data opaque) int64_t sleeptime_ns, endtime_ns; if (!cpu_throttle_get_percentage()) { +qatomic_set(&cpu->throttle_thread_scheduled, 0); return; } -- 2.27.0
Re: [PATCH] target/arm: fix s2mmu input size check
On 2022/05/06 1:13, Richard Henderson wrote: On 5/4/22 22:12, m...@sfc.wide.ad.jp wrote: From: Keisuke Iida The maximum IPA size('inputsize') is constrained by the implemented PA size that is specified by ID_AA64MMFR0_EL1.PARange. Please reference Arm Architecture Reference Manual for A-profile architecture "Supported IPA size" on page D5-4788. Signed-off-by: Keisuke Iida --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index 5a244c3ed9..868e7a2c0b 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6,7 +6,7 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level, } /* Inputsize checks. */ - if (inputsize > outputsize && + if (inputsize > arm_pamax(cpu) && This is incorrect -- arm_pamax has already been taken into account in computing outputsize. There are many more constraints than just this. You need to have a look at the computation of ps and tsz in aa64_va_parameters, and then the computation of outputsize near the beginning of get_phys_addr_lpae, which takes arm_pamax into account by bounding ps against ID_AA64MMFR0.PARANGE, and pamax_map. What problem are you encountering? Address Translation Fault is triggered when PA size set by VTCR_EL2.PS is less than IPA size set by VTCR_EL2.T0SZ on the guest. (e.g. vtcr_el2.PS = 1 && vtcr_el2.T0SZ = 25. PA size is 36bit, and IPA size is 39bit.) In this case, inputsize = 39 and outputsize = 36, so inputsize > outputsize is true and a fault is triggered. This behavior means that the effective minimum VTCR_EL2.T0SZ value depends on VTCR_EL2.PS set by guest. Is this the expected behavior for qemu? As a side note, this does not happen before qemu 6.2. r~ -- Keisuke Iida
Re: Possible bug in Aarch64 single-stepping
On 7. May 2022, at 15:42, Chris Howard wrote: > > Hi, I’m writing a simple debugger in assembly code for the Raspberry Pi 3B > (in aarch64). > > I’m using QEMU 7.0.0. Everything is running in EL1. (I have MDE and KDE set > in MDSCR_EL1). > > I’m coming across Unexpected Behaviour when playing with single-stepping: > > It appears that single-stepping is enabled (ie. an exception is generated > after every instruction) when the SS bit (bit-0) is set in MDSCR_EL1 and > debug events are enabled in the CPSR (“D” bit clear) *** irrespective of > whether the SS bit (bit-21) is set in CPSR or not ***. > > I thought the SS bit (bit-21) needs to be set in CPSR for single-stepping to > occur (and that it gets cleared whenever an exception is taken and needs to > be reset if one wants to single-step again). > > Have I misunderstood / misconfigured something, or is this a bug? > > Attached is a minimal(ish) example: Oh, and the exception occurs immediately (after the ERET), rather than after the instruction has been executed. It appears to be acting like a hardware breakpoint. PS. In plain gdb (ie. no nice user interface) a large number (but not all) of the system registers gets displayed after each step. It would be nice if these were sorted in some way. At the moment they’re completely jumbled — not alphabetic, not grouped by EL, nor by “meaning” (DBGWVR0_EL1 isn’t necessarily next to DBGWCR0_EL1). Also, there are multiple (identical?) instances of “DBGBVR” and “DBGBCR” (and “DBGWVR” and “DBGWCR”) rather than the expected “DBGWVR0_EL1”, “DBGWVR1_EL1” etc. Would this be a QEMU or a GDB issue? Or isn’t it an issue at all? :-)
Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
On 4/29/22 05:07, Xiaojuan Yang wrote: +int ipmap_mask = 0xff << ipmap_offset; ... +int cpu_mask = 0xff << ipmap_offset; These two masks are redundant with +ipnum = ((s->ipmap[ipmap_index] & ipmap_mask) >> ipmap_offset) & 0xf; ... +cpu = ((s->coremap[cpu_index] & cpu_mask) >> cpu_offset) & 0xf; the 0xf masking here. +cpu = ctz32(cpu); +cpu = (cpu >= 4) ? 0 : cpu; You are not considering CSR[0x420][49], which changes the format of this mapping. I think this function is wrong because you maintain an unmapped enable bitmap, but you do not maintain an unmapped status bitmap, which *should* be readable from EXTIOI_ISR_{START,END}, but is not present in extioi_readw. I think that only extioi_setirq should actually change the unmapped status bitmap, and that extioi_update_irq should only evaluate the mapping to apply changes to the cpus. +if (level) { +/* if not enable return false */ +if (((s->enable[enable_index]) & (1 << enable_mask)) == 0) { +return; +} +s->coreisr[cpu][coreisr_index] |= (1 << coreisr_mask); +qemu_set_irq(s->parent_irq[cpu][ipnum], level); +} else { +s->coreisr[cpu][coreisr_index] &= ~(1 << coreisr_mask); +qemu_set_irq(s->parent_irq[cpu][ipnum], level); +} This final bit, updating the cpu irq is also wrong, in that it should be unconditional. This is the only way that it will work for the usage in updating the enable mask. I think you are not considering when the MAP registers overlap outputs. For instance, if all 256 bits of EXT_IOIMap contain 0, then all of EXT_IOI[n*32+31 : n*32] overlap. When that happens, you cannot lower the level of the cpu pin until all of the matching ioi interrupts are low. Or, perhaps I don't understand how this is supposed to work? The documentation is very weak. +static void extioi_writew(void *opaque, hwaddr addr, + uint64_t val, unsigned size) +{ +LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque); +int cpu, index, old_data, data_offset; +uint32_t offset; +trace_loongarch_extioi_writew(size, (uint32_t)addr, val); + +offset = addr & 0x; + +switch (offset) { +case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1: +index = (offset - EXTIOI_NODETYPE_START) >> 2; +s->nodetype[index] = val; +break; +case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1: +index = (offset - EXTIOI_IPMAP_START) >> 2; +s->ipmap[index] = val; +break; Do you need to recompute the entire interrupt map when ipmap changes? +case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1: +index = (offset - EXTIOI_ENABLE_START) >> 2; +old_data = s->enable[index]; +if (old_data != (int)val) { +s->enable[index] = val; +old_data = old_data ^ val; +data_offset = ctz32(old_data); +while (data_offset != 32) { +if (!(val & (1 << data_offset))) { +extioi_update_irq(s, data_offset + index * 32, 0); This is not correct -- you're unconditionally setting level=0, corrupting the old value of coreisr[cpu][index]. You need to recompute *without* changning those levels. +case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1: +index = (offset - EXTIOI_COREMAP_START) >> 2; +s->coremap[index] = val; +break; Recompute the entire interrupt map? r~
Re: [PATCH v9 10/17] vfio-user: run vfio-user context
> On May 6, 2022, at 1:44 AM, Markus Armbruster wrote: > > Jag Raman writes: > >>> On May 5, 2022, at 10:42 AM, Markus Armbruster wrote: >>> >>> Jag Raman writes: >>> > On May 5, 2022, at 3:44 AM, Markus Armbruster wrote: > > Jag Raman writes: > >>> On May 4, 2022, at 7:42 AM, Markus Armbruster wrote: >>> >>> Jagannathan Raman writes: >>> Setup a handler to run vfio-user context. The context is driven by messages to the file descriptor associated with it - get the fd for the context and hook up the handler with it Signed-off-by: Elena Ufimtseva Signed-off-by: John G Johnson Signed-off-by: Jagannathan Raman Reviewed-by: Stefan Hajnoczi >>> >>> [...] >>> @@ -164,6 +172,76 @@ static void vfu_object_set_device(Object *obj, const char *str, Error **errp) vfu_object_init_ctx(o, errp); } +static void vfu_object_ctx_run(void *opaque) +{ + VfuObject *o = opaque; + const char *vfu_id; + char *vfu_path, *pci_dev_path; + int ret = -1; + + while (ret != 0) { + ret = vfu_run_ctx(o->vfu_ctx); + if (ret < 0) { + if (errno == EINTR) { + continue; + } else if (errno == ENOTCONN) { + vfu_id = object_get_canonical_path_component(OBJECT(o)); + vfu_path = object_get_canonical_path(OBJECT(o)); >>> >>> Hmm. @vfu_id is always the last component of @vfu_path. Why do we need >>> to send both? >> >> vfu_id is the ID that the user/Orchestrator passed as a command-line >> option >> during addition/creation. So it made sense to report back with the same >> ID >> that they used. But I’m OK with dropping this if that’s what you prefer. > > Matter of taste, I guess. I'd drop it simply to saves us the trouble of > documenting it. > > If we decide to keep it, then I think we should document it's always the > last component of @vfu_path. > + g_assert(o->pci_dev); + pci_dev_path = object_get_canonical_path(OBJECT(o->pci_dev)); + qapi_event_send_vfu_client_hangup(vfu_id, vfu_path, + o->device, pci_dev_path); >>> >>> Where is o->device set? I'm asking because I it must not be null here, >>> and that's not locally obvious. >> >> Yeah, it’s not obvious from this patch that o->device is guaranteed to be >> non-NULL. It’s set by vfu_object_set_device(). Please see the following >> patches in the series: >> vfio-user: define vfio-user-server object >> vfio-user: instantiate vfio-user context > > vfu_object_set_device() is a QOM property setter. It gets called if and > only if the property is set. If it's never set, ->device remains null. > What ensures it's always set? That’s a good question - it’s not obvious from this patch. The code would not reach here if o->device is not set. If o->device is NULL, vfu_object_init_ctx() would bail out early without setting up vfu_object_attach_ctx() and vfu_object_ctx_run() (this function) handlers. >>> >>> Yes: >>> >>> static void vfu_object_init_ctx(VfuObject *o, Error **errp) >>> { >>> ERRP_GUARD(); >>> DeviceState *dev = NULL; >>> vfu_pci_type_t pci_type = VFU_PCI_TYPE_CONVENTIONAL; >>> int ret; >>> >>> if (o->vfu_ctx || !o->socket || !o->device || >>> !phase_check(PHASE_MACHINE_READY)) { >>> return; >>> } >>> >>> Bails out without setting an error. Sure that's appropriate? >> >> It’s not an error per se - vfu_object_init_ctx() doesn’t proceed >> further if device/socket is not set or if the machine is not ready. >> >> Both socket and device are required properties, so they would >> eventually be set by vfu_object_set_socket() / >> vfu_object_set_device() - and these setters eventually kick >> vfu_object_init_ctx(). > > Early returns from a function that sets error on failure triggers bug > spider sense, because forgetting to set an error on failure is a fairly > common mistake. > > The root of the problem is of course that the function's contract is not > obvious. The name vfu_object_init_ctx() suggests it initializes > something. But it clearly doesn't unless certain conditions are met. > The reader is left to wonder whether that's a bug, or whether that's > what it is supposed to do. > > A function contract spelling out when the function is supposed to do > what (including "nothing") would help. Sure, will add a comment explaining what this function is supposed to do. -- Jag > > [...] >
[PATCH 00/17] powernv: introduce pnv-phb unified devices
Hi, Since the 7.0.0 release cycle we have a desire to use the powernv emulation with libvirt. To do that we need to enable user creatable pnv-phb devices to allow more user customization an to avoid spamming multiple default devices in the domain XML. In the middle of the previous cycle we experimented with user created pnv-phb3/pnv-phb3-root-port/pnv-phb4/pnv-phb4-root-port/pnv-phb5. The end result, although functional, is that the user needs to deal with a lot of versioned devices that, from the user perspective, does the same thing. In a way we outsourced the implementation details of the PHBs (e.g. pnv8 uses phb3, pnv9 uses phb4) to the command line. Having more devices also puts an extra burden in the libvirt support we want to have. To solve this, Cedric and Frederic gave the idea of adding a common virtual pnv-phb device that the user can add in the command line, and QEMU handles the details internally. Unfortunatelly this idea turned out to be way harder than anticipated. Between creating a device that would just forward the callbacks to the existing devices internally, creating a PnvPHB device with the minimal attributes and making the other devices inherit from it, and making an 'abstract' device that could be casted for both phb3 and phb4 PHBs, all sorts of very obscure problems occured: PHBs not being detected, interrupts not being delivered and memory regions not being able to read/write registers. My initial impression is that there are assumptions made both in ppc/pnv and hw/pci-host that requires the memory region and the bus being in the same device. Even if we somehow figure all this out, the resulting code is hacky and annoying to maitain. This brings us to this series. The cleaner way I found to accomplish what we want to do is to create real, unified pnv-phb/phb-phb-root-port devices, and get rid of all the versioned ones. This is done by merging all the PHB3/PHB4 attributes in unified devices. pnv_phb3* and pnv_phb4* files end up using the same pnv-phb and phb-phb-root-port unified devices, with the difference that pnv_phb3* only cares about version 3 attributes and pnv_phb4* only cares about PHB4 attributes. Introducing new PHB versions in the future will be a matter of adding any non-existent attributes in the unified pnv-phb* devices and using them in separated pnv_phbN* files. The pnv-phb implementation per se is just a router for either phb3 or phb4 logic, done in init() and realize() time, after we check which powernv machine we're running. If running with powernv8 we redirect control to pnv_phb3_realize(), otherwise we redirect to pnv_phb4_realize(). The unified device does not do anything per se other than handling control to the right version. After this series, this same powernv8 command line that boots a powernv8 machine with some phbs and root ports and with network: ./qemu-system-ppc64 -m 4G \ -machine powernv8 -smp 2,sockets=2,cores=1,threads=1 \ -accel tcg,thread=multi -bios skiboot.lid \ -kernel vmlinux -initrd buildroot.rootfs.cpio -append 'console=hvc0 ro xmon=on' \ -nodefaults -serial mon:stdio -nographic \ -device pnv-phb,chip-id=0,index=0,id=pcie.0 \ -device pnv-phb,chip-id=0,index=1,id=pcie.1 \ -device pnv-phb,chip-id=1,index=2,id=pcie.2 \ -device pnv-phb-root-port,id=root0,bus=pcie.2 \ -device pnv-phb-root-port,id=root1,bus=pcie.1 \ -device pcie-pci-bridge,id=bridge1,bus=root0,addr=0x0 \ -device nvme,bus=bridge1,addr=0x1,drive=drive0,serial=1234 \ -drive file=./simics-disk.raw,if=none,id=drive0,format=raw,cache=none \ -device e1000e,netdev=net0,mac=C0:ff:EE:00:01:04,bus=bridge1,addr=0x3 \ -netdev bridge,helper=/usr/libexec/qemu-bridge-helper,br=virbr0,id=net0 \ -device nec-usb-xhci,bus=bridge1,addr=0x2 Can be used to boot powernv9 and powernv10 machines with the same attributes just by changing the machine type. Daniel Henrique Barboza (17): ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2* ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[] ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces ppc/pnv: add unified pnv-phb header ppc/pnv: add pnv-phb device ppc/pnv: remove PnvPHB3 ppc/pnv: user created pnv-phb for powernv8 ppc/pnv: remove PnvPHB4 ppc/pnv: user creatable pnv-phb for powernv9 ppc/pnv: use PnvPHB.version ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs ppc/pnv: user creatable pnv-phb for powernv10 ppc/pnv: add pnv_phb_get_current_machine() ppc/pnv: add pnv-phb-root-port device ppc/pnv: remove pnv-phb3-root-port ppc/pnv: remove pnv-phb4-root-port ppc/pnv: remove pecc->rp_model hw/pci-host/meson.build| 3 +- hw/pci-host/pnv_phb.c | 258 hw/pci-host/pnv_phb3.c | 256 +++- hw/pci-host/pnv_phb3_msi.c | 12 +- hw/pci-host/pnv_phb3_pbcq.c| 8 +- hw/pci-host/pnv_phb4.c | 298 - hw/pci-host/pnv_phb4_pec.c | 14 +- hw/ppc/pnv.c | 41
[PATCH 01/17] ppc/pnv: rename PnvPHB3.ioda* to PnvPHB3.ioda2*
We're going to merge all the existing pnv-phb models into a single pnv-phb model. Users will be able to add phbs by using the same pnv-phb device, regardless of which powernv machine is being used, and internally we'll handle which PHB version the device needs to have. The unified pnv-phb model needs to be usable by the existing pnv_phb3 and pnv_phb4 code base. One way of accomplishing that is to merge PnvPHB3 and PnvPHB4 into a single PnvPHB struct. To do that we need to handle the cases where the same attribute might have different meaning/semantics depending on the version. One of these attributes is the 'ioda' arrays. This patch renames PnvPHB3.ioda* arrays to PnvPHB3.ioda2*. The reason why we're calling 'ioda2' is because PnvPHB3 uses IODA version 2. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb3.c | 18 +- include/hw/pci-host/pnv_phb3.h | 12 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 3f03467dde..860f8846df 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -165,7 +165,7 @@ static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index) } /* Get table entry */ -m64 = phb->ioda_M64BT[index]; +m64 = phb->ioda2_M64BT[index]; if (!(m64 & IODA2_M64BT_ENABLE)) { return; @@ -215,7 +215,7 @@ static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val) { uint8_t server, prio; -phb->ioda_LXIVT[idx] = val & (IODA2_LXIVT_SERVER | +phb->ioda2_LXIVT[idx] = val & (IODA2_LXIVT_SERVER | IODA2_LXIVT_PRIORITY | IODA2_LXIVT_NODE_ID); server = GETFIELD(IODA2_LXIVT_SERVER, val); @@ -241,11 +241,11 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, switch (table) { case IODA2_TBL_LIST: -tptr = phb->ioda_LIST; +tptr = phb->ioda2_LIST; mask = 7; break; case IODA2_TBL_LXIVT: -tptr = phb->ioda_LXIVT; +tptr = phb->ioda2_LXIVT; mask = 7; break; case IODA2_TBL_IVC_CAM: @@ -263,7 +263,7 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, mask = 255; break; case IODA2_TBL_TVT: -tptr = phb->ioda_TVT; +tptr = phb->ioda2_TVT; mask = 511; break; case IODA2_TBL_TCAM: @@ -271,15 +271,15 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, mask = 63; break; case IODA2_TBL_M64BT: -tptr = phb->ioda_M64BT; +tptr = phb->ioda2_M64BT; mask = 15; break; case IODA2_TBL_M32DT: -tptr = phb->ioda_MDT; +tptr = phb->ioda2_MDT; mask = 255; break; case IODA2_TBL_PEEV: -tptr = phb->ioda_PEEV; +tptr = phb->ioda2_PEEV; mask = 3; break; default: @@ -869,7 +869,7 @@ static IOMMUTLBEntry pnv_phb3_translate_iommu(IOMMUMemoryRegion *iommu, } /* Choose TVE XXX Use PHB3 Control Register */ tve_sel = (addr >> 59) & 1; -tve = ds->phb->ioda_TVT[ds->pe_num * 2 + tve_sel]; +tve = ds->phb->ioda2_TVT[ds->pe_num * 2 + tve_sel]; pnv_phb3_translate_tve(ds, addr, flag & IOMMU_WO, tve, &ret); break; case 01: diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h index af6ec83cf6..73da31fbd2 100644 --- a/include/hw/pci-host/pnv_phb3.h +++ b/include/hw/pci-host/pnv_phb3.h @@ -141,12 +141,12 @@ struct PnvPHB3 { MemoryRegion pci_mmio; MemoryRegion pci_io; -uint64_t ioda_LIST[8]; -uint64_t ioda_LXIVT[8]; -uint64_t ioda_TVT[512]; -uint64_t ioda_M64BT[16]; -uint64_t ioda_MDT[256]; -uint64_t ioda_PEEV[4]; +uint64_t ioda2_LIST[8]; +uint64_t ioda2_LXIVT[8]; +uint64_t ioda2_TVT[512]; +uint64_t ioda2_M64BT[16]; +uint64_t ioda2_MDT[256]; +uint64_t ioda2_PEEV[4]; uint32_t total_irq; ICSState lsis; -- 2.32.0
[PATCH 04/17] ppc/pnv: add unified pnv-phb header
This patch adds the pnv_phb.h header with the declarations we're going to use in this soon to be added device. It consists of an union between all attributes of PnvPHB3 and PnvPHB4 devices. This will allow for the same PnvPHB device to be used for PHB3 and PHB4 code. Some struct definitions from PnvPHB3 had to be moved to the new header due to scope constraints. Signed-off-by: Daniel Henrique Barboza --- include/hw/pci-host/pnv_phb.h | 211 + include/hw/pci-host/pnv_phb3.h | 65 +- 2 files changed, 212 insertions(+), 64 deletions(-) create mode 100644 include/hw/pci-host/pnv_phb.h diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h new file mode 100644 index 00..2a8bf9a66d --- /dev/null +++ b/include/hw/pci-host/pnv_phb.h @@ -0,0 +1,211 @@ +/* + * QEMU PowerPC PowerNV Unified PHB model + * + * Copyright (c) 2022, IBM Corporation. + * + * This code is licensed under the GPL version 2 or later. See the + * COPYING file in the top-level directory. + */ + +#ifndef PCI_HOST_PNV_PHB_H +#define PCI_HOST_PNV_PHB_H + +#include "hw/pci/pcie_host.h" +#include "hw/pci/pcie_port.h" +#include "hw/ppc/xics.h" +#include "hw/ppc/xive.h" +#include "qom/object.h" + +/* pnv_phb3.h types */ +#define PNV_PHB3_NUM_M64 16 +#define PNV_PHB3_NUM_REGS (0x1000 >> 3) +#define PHB3_MAX_MSI 2048 + +typedef struct PnvPHB3 PnvPHB3; +typedef struct PnvChip PnvChip; + +typedef struct Phb3MsiState { +ICSState ics; +qemu_irq *qirqs; + +PnvPHB3 *phb; +uint64_t rba[PHB3_MAX_MSI / 64]; +uint32_t rba_sum; +} Phb3MsiState; + +typedef struct PnvPBCQState { +DeviceState parent; + +uint32_t nest_xbase; +uint32_t spci_xbase; +uint32_t pci_xbase; +#define PBCQ_NEST_REGS_COUNT0x46 +#define PBCQ_PCI_REGS_COUNT 0x15 +#define PBCQ_SPCI_REGS_COUNT0x5 + +uint64_t nest_regs[PBCQ_NEST_REGS_COUNT]; +uint64_t spci_regs[PBCQ_SPCI_REGS_COUNT]; +uint64_t pci_regs[PBCQ_PCI_REGS_COUNT]; +MemoryRegion mmbar0; +MemoryRegion mmbar1; +MemoryRegion phbbar; +uint64_t mmio0_base; +uint64_t mmio0_size; +uint64_t mmio1_base; +uint64_t mmio1_size; +PnvPHB3 *phb; + +MemoryRegion xscom_nest_regs; +MemoryRegion xscom_pci_regs; +MemoryRegion xscom_spci_regs; +} PnvPBCQState; + +/* + * We have one such address space wrapper per possible device under + * the PHB since they need to be assigned statically at qemu device + * creation time. The relationship to a PE is done later dynamically. + * This means we can potentially create a lot of these guys. Q35 + * stores them as some kind of radix tree but we never really need to + * do fast lookups so instead we simply keep a QLIST of them for now, + * we can add the radix if needed later on. + * + * We do cache the PE number to speed things up a bit though. + */ +typedef struct PnvPhb3DMASpace { +PCIBus *bus; +uint8_t devfn; +int pe_num; /* Cached PE number */ +#define PHB_INVALID_PE (-1) +PnvPHB3 *phb; +AddressSpace dma_as; +IOMMUMemoryRegion dma_mr; +MemoryRegion msi32_mr; +MemoryRegion msi64_mr; +QLIST_ENTRY(PnvPhb3DMASpace) list; +} PnvPhb3DMASpace; + +/* pnv_phb4.h types */ +#define PNV_PHB4_MAX_LSIs 8 +#define PNV_PHB4_MAX_INTs 4096 +#define PNV_PHB4_MAX_MIST (PNV_PHB4_MAX_INTs >> 2) +#define PNV_PHB4_MAX_MMIO_WINDOWS 32 +#define PNV_PHB4_MIN_MMIO_WINDOWS 16 +#define PNV_PHB4_NUM_REGS (0x3000 >> 3) +#define PNV_PHB4_MAX_PEs 512 +#define PNV_PHB4_MAX_TVEs (PNV_PHB4_MAX_PEs * 2) +#define PNV_PHB4_MAX_PEEVs (PNV_PHB4_MAX_PEs / 64) +#define PNV_PHB4_MAX_MBEs (PNV_PHB4_MAX_MMIO_WINDOWS * 2) +typedef struct PnvPhb4PecState PnvPhb4PecState; + +/* + * Unified PHB PCIe Host Bridge for PowerNV machines + */ +typedef struct PnvPHB PnvPHB; +#define TYPE_PNV_PHB "pnv-phb" +OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB) + +struct PnvPHB { +PCIExpressHost parent_obj; + +uint32_t chip_id; +uint32_t phb_id; +char bus_path[8]; +MemoryRegion pci_mmio; +MemoryRegion pci_io; +qemu_irq *qirqs; + +/* + * PnvPHB3 attributes + */ +uint64_t regs3[PNV_PHB3_NUM_REGS]; +MemoryRegion mr_regs3; + +MemoryRegion mr_m32; +MemoryRegion mr_m64[PNV_PHB3_NUM_M64]; + +uint64_t ioda2_LIST[8]; +uint64_t ioda2_LXIVT[8]; +uint64_t ioda2_TVT[512]; +uint64_t ioda2_M64BT[16]; +uint64_t ioda2_MDT[256]; +uint64_t ioda2_PEEV[4]; + +uint32_t total_irq; +ICSState lsis; +Phb3MsiState msis; + +PnvPBCQState pbcq; + +QLIST_HEAD(, PnvPhb3DMASpace) v3_dma_spaces; + +PnvChip *chip; + +/* + * PnvPHB4 attributes + */ +uint64_t version; + +/* The owner PEC */ +PnvPhb4PecState *pec; + +/* Main register images */ +uint64_t regs[PNV_PHB4_NUM_REGS]; +MemoryRegion mr_regs; + +/* Extra SCOM-only register */ +uint64_t scom_hv_ind_addr_reg; + +/
[PATCH 03/17] ppc/pnv: rename PnvPHB3.dma_spaces to PnvPHB3.v3_dma_spaces
The last common attribute that has a different meaning/semantic between PnvPHB3 and PnvPHB4 devices is the 'dma_spaces' QLIST. Rename the PHB3 version to 'v3_dma_spaces'. The reason why we chose that instead of 'dma3_spaces' or similar is to avoid any misunderstanding about this being related to DMA version 3. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb3.c | 10 +- include/hw/pci-host/pnv_phb3.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 77ee2325be..70d92edd94 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -421,7 +421,7 @@ static void pnv_phb3_rtc_invalidate(PnvPHB3 *phb, uint64_t val) PnvPhb3DMASpace *ds; /* Always invalidate all for now ... */ -QLIST_FOREACH(ds, &phb->dma_spaces, list) { +QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) { ds->pe_num = PHB_INVALID_PE; } } @@ -460,7 +460,7 @@ static void pnv_phb3_update_all_msi_regions(PnvPHB3 *phb) { PnvPhb3DMASpace *ds; -QLIST_FOREACH(ds, &phb->dma_spaces, list) { +QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) { pnv_phb3_update_msi_regions(ds); } } @@ -938,7 +938,7 @@ static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn) PnvPHB3 *phb = opaque; PnvPhb3DMASpace *ds; -QLIST_FOREACH(ds, &phb->dma_spaces, list) { +QLIST_FOREACH(ds, &phb->v3_dma_spaces, list) { if (ds->bus == bus && ds->devfn == devfn) { break; } @@ -961,7 +961,7 @@ static AddressSpace *pnv_phb3_dma_iommu(PCIBus *bus, void *opaque, int devfn) ds, "msi64", 0x10); pnv_phb3_update_msi_regions(ds); -QLIST_INSERT_HEAD(&phb->dma_spaces, ds, list); +QLIST_INSERT_HEAD(&phb->v3_dma_spaces, ds, list); } return &ds->dma_as; } @@ -970,7 +970,7 @@ static void pnv_phb3_instance_init(Object *obj) { PnvPHB3 *phb = PNV_PHB3(obj); -QLIST_INIT(&phb->dma_spaces); +QLIST_INIT(&phb->v3_dma_spaces); /* LSI sources */ object_initialize_child(obj, "lsi", &phb->lsis, TYPE_ICS); diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h index 486dbbefee..35483e59c3 100644 --- a/include/hw/pci-host/pnv_phb3.h +++ b/include/hw/pci-host/pnv_phb3.h @@ -155,7 +155,7 @@ struct PnvPHB3 { PnvPBCQState pbcq; -QLIST_HEAD(, PnvPhb3DMASpace) dma_spaces; +QLIST_HEAD(, PnvPhb3DMASpace) v3_dma_spaces; PnvChip *chip; }; -- 2.32.0
[PATCH 02/17] ppc/pnv: rename PnvPHB3.regs[] to PnvPHB3.regs3[]
Another redundancy between PnvPHB3 and PnvPHB4 that we should take care before merging all together in an unified model is the regs[] array. Rename the regs[] array used by the PHB3 code to 'regs3'. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb3.c | 54 +- hw/pci-host/pnv_phb3_msi.c | 6 ++-- include/hw/pci-host/pnv_phb3.h | 4 +-- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 860f8846df..77ee2325be 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -27,7 +27,7 @@ static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb) { PCIHostState *pci = PCI_HOST_BRIDGE(phb); -uint64_t addr = phb->regs[PHB_CONFIG_ADDRESS >> 3]; +uint64_t addr = phb->regs3[PHB_CONFIG_ADDRESS >> 3]; uint8_t bus, devfn; if (!(addr >> 63)) { @@ -53,7 +53,7 @@ static void pnv_phb3_config_write(PnvPHB3 *phb, unsigned off, if (!pdev) { return; } -cfg_addr = (phb->regs[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc; +cfg_addr = (phb->regs3[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc; cfg_addr |= off; limit = pci_config_size(pdev); if (limit <= cfg_addr) { @@ -89,7 +89,7 @@ static uint64_t pnv_phb3_config_read(PnvPHB3 *phb, unsigned off, if (!pdev) { return ~0ull; } -cfg_addr = (phb->regs[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc; +cfg_addr = (phb->regs3[PHB_CONFIG_ADDRESS >> 3] >> 32) & 0xffc; cfg_addr |= off; limit = pci_config_size(pdev); if (limit <= cfg_addr) { @@ -122,14 +122,14 @@ static void pnv_phb3_check_m32(PnvPHB3 *phb) memory_region_del_subregion(phb->mr_m32.container, &phb->mr_m32); } -if (!(phb->regs[PHB_PHB3_CONFIG >> 3] & PHB_PHB3C_M32_EN)) { +if (!(phb->regs3[PHB_PHB3_CONFIG >> 3] & PHB_PHB3C_M32_EN)) { return; } /* Grab geometry from registers */ -base = phb->regs[PHB_M32_BASE_ADDR >> 3]; -start = phb->regs[PHB_M32_START_ADDR >> 3]; -size = ~(phb->regs[PHB_M32_BASE_MASK >> 3] | 0xfffcull) + 1; +base = phb->regs3[PHB_M32_BASE_ADDR >> 3]; +start = phb->regs3[PHB_M32_START_ADDR >> 3]; +size = ~(phb->regs3[PHB_M32_BASE_MASK >> 3] | 0xfffcull) + 1; /* Check if it matches an enabled MMIO region in the PBCQ */ if (memory_region_is_mapped(&pbcq->mmbar0) && @@ -179,7 +179,7 @@ static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index) size = GETFIELD(IODA2_M64BT_MASK, m64) << 20; size |= 0xfffcull; size = ~size + 1; -start = base | (phb->regs[PHB_M64_UPPER_BITS >> 3]); +start = base | (phb->regs3[PHB_M64_UPPER_BITS >> 3]); /* Check if it matches an enabled MMIO region in the PBCQ */ if (memory_region_is_mapped(&pbcq->mmbar0) && @@ -233,7 +233,7 @@ static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val) static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, unsigned *out_table, unsigned *out_idx) { -uint64_t adreg = phb->regs[PHB_IODA_ADDR >> 3]; +uint64_t adreg = phb->regs3[PHB_IODA_ADDR >> 3]; unsigned int index = GETFIELD(PHB_IODA_AD_TADR, adreg); unsigned int table = GETFIELD(PHB_IODA_AD_TSEL, adreg); unsigned int mask; @@ -300,7 +300,7 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, index = (index + 1) & mask; adreg = SETFIELD(PHB_IODA_AD_TADR, adreg, index); } -phb->regs[PHB_IODA_ADDR >> 3] = adreg; +phb->regs3[PHB_IODA_ADDR >> 3] = adreg; return tptr; } @@ -364,7 +364,7 @@ void pnv_phb3_remap_irqs(PnvPHB3 *phb) } /* Grab local LSI source ID */ -local = GETFIELD(PHB_LSI_SRC_ID, phb->regs[PHB_LSI_SOURCE_ID >> 3]) << 3; +local = GETFIELD(PHB_LSI_SRC_ID, phb->regs3[PHB_LSI_SOURCE_ID >> 3]) << 3; /* Grab global one and compare */ global = GETFIELD(PBCQ_NEST_LSI_SRC, @@ -412,7 +412,7 @@ static void pnv_phb3_lsi_src_id_write(PnvPHB3 *phb, uint64_t val) { /* Sanitize content */ val &= PHB_LSI_SRC_ID; -phb->regs[PHB_LSI_SOURCE_ID >> 3] = val; +phb->regs3[PHB_LSI_SOURCE_ID >> 3] = val; pnv_phb3_remap_irqs(phb); } @@ -429,7 +429,7 @@ static void pnv_phb3_rtc_invalidate(PnvPHB3 *phb, uint64_t val) static void pnv_phb3_update_msi_regions(PnvPhb3DMASpace *ds) { -uint64_t cfg = ds->phb->regs[PHB_PHB3_CONFIG >> 3]; +uint64_t cfg = ds->phb->regs3[PHB_PHB3_CONFIG >> 3]; if (cfg & PHB_PHB3C_32BIT_MSI_EN) { if (!memory_region_is_mapped(&ds->msi32_mr)) { @@ -501,16 +501,16 @@ void pnv_phb3_reg_write(void *opaque, hwaddr off, uint64_t val, unsigned size) break; /* LEM stuff */ case PHB_LEM_FIR_AND_MASK: -phb->regs[PHB_LEM_FIR_ACCUM >> 3] &= val; +phb->regs3[PHB_LEM_FIR_ACCUM >> 3] &= val; return; case PHB_LEM_FIR_OR_MASK: -phb->regs[PHB_LEM_FIR_ACCUM >> 3] |= val; +phb->regs
[PATCH 05/17] ppc/pnv: add pnv-phb device
This device works as a generic pnv-phb that redirects the control flow to one of the implemented PHB versions (PHB3 and PHB4 at this moment). The control redirection happens in the instance_init() and realize() callbacks, where we check which powernv machine we're running and execute the PnvPHB3 callbacks if running powernv8 or PnvPHB4 if running powernv9/10. This will allow us to keep the existing PHB3 and PHB4 code as is, just changing their device type to PnvPHB3/PnvPHB4 to PnvPHB when we're ready. For now we're putting logic to handle the PHB3 version. We'll add PHB4 later on. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/meson.build| 3 +- hw/pci-host/pnv_phb.c | 116 + hw/pci-host/pnv_phb3.c | 4 +- include/hw/pci-host/pnv_phb3.h | 3 + 4 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 hw/pci-host/pnv_phb.c diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build index 4c4f39c15c..b4107b7a2a 100644 --- a/hw/pci-host/meson.build +++ b/hw/pci-host/meson.build @@ -32,5 +32,6 @@ specific_ss.add(when: 'CONFIG_PCI_POWERNV', if_true: files( 'pnv_phb3_msi.c', 'pnv_phb3_pbcq.c', 'pnv_phb4.c', - 'pnv_phb4_pec.c' + 'pnv_phb4_pec.c', + 'pnv_phb.c', )) diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c new file mode 100644 index 00..3dd08f768f --- /dev/null +++ b/hw/pci-host/pnv_phb.c @@ -0,0 +1,116 @@ +/* + * QEMU PowerPC PowerNV Unified PHB model + * + * Copyright (c) 2022, IBM Corporation. + * + * This code is licensed under the GPL version 2 or later. See the + * COPYING file in the top-level directory. + */ +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qapi/visitor.h" +#include "qapi/error.h" +#include "hw/pci-host/pnv_phb.h" +#include "hw/pci/pcie_host.h" +#include "hw/pci/pcie_port.h" +#include "hw/ppc/pnv.h" +#include "hw/irq.h" +#include "hw/qdev-properties.h" +#include "qom/object.h" +#include "sysemu/sysemu.h" + + +static char *pnv_phb_get_chip_typename(void) +{ +Object *qdev_machine = qdev_get_machine(); +PnvMachineState *pnv = PNV_MACHINE(qdev_machine); +MachineState *machine = MACHINE(pnv); +g_autofree char *chip_typename = NULL; +int i; + +if (!machine->cpu_type) { +return NULL; +} + +i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX); +chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"), +i, machine->cpu_type); + +return g_steal_pointer(&chip_typename); +} + +static void pnv_phb_instance_init(Object *obj) +{ +g_autofree char *chip_typename = pnv_phb_get_chip_typename(); + +/* + * When doing command line instrospection we won't have + * a valid machine->cpu_type value. + */ +if (!chip_typename) { +return; +} + +if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) || +!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) || +!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { +pnv_phb3_instance_init(obj); +} +} + +static void pnv_phb_realize(DeviceState *dev, Error **errp) +{ +g_autofree char *chip_typename = pnv_phb_get_chip_typename(); + +g_assert(chip_typename != NULL); + +if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) || +!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) || +!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { +/* PnvPHB3 */ +pnv_phb3_realize(dev, errp); +} +} + +static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge, + PCIBus *rootbus) +{ +PnvPHB *phb = PNV_PHB(host_bridge); + +snprintf(phb->bus_path, sizeof(phb->bus_path), "00%02x:%02x", + phb->chip_id, phb->phb_id); +return phb->bus_path; +} + +static Property pnv_phb_properties[] = { +DEFINE_PROP_UINT32("index", PnvPHB, phb_id, 0), +DEFINE_PROP_UINT32("chip-id", PnvPHB, chip_id, 0), +DEFINE_PROP_END_OF_LIST(), +}; + +static void pnv_phb_class_init(ObjectClass *klass, void *data) +{ +PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); +DeviceClass *dc = DEVICE_CLASS(klass); + +hc->root_bus_path = pnv_phb_root_bus_path; +dc->realize = pnv_phb_realize; +device_class_set_props(dc, pnv_phb_properties); +set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); +dc->user_creatable = true; +} + +static const TypeInfo pnv_phb_type_info = { +.name = TYPE_PNV_PHB, +.parent= TYPE_PCIE_HOST_BRIDGE, +.instance_size = sizeof(PnvPHB), +.class_init= pnv_phb_class_init, +.instance_init = pnv_phb_instance_init, +}; + +static void pnv_phb_register_types(void) +{ +type_register_static(&pnv_phb_type_info); +} + +type_init(pnv_phb_register_types) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 70d92edd94..8e31a69083 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -966,7 +966,7 @@ static AddressSpace *pnv_phb3_d
[PATCH 10/17] ppc/pnv: use PnvPHB.version
The 'version' attribute of the PnvPHB was never used. Instead of removing it, let's make use of it by setting the PHB version the PnvPHB device is currently running. This distinction will be used next patch. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb.c | 11 +++ include/hw/pci-host/pnv_phb.h | 7 +-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c index 9583c703d4..cef6a57d50 100644 --- a/hw/pci-host/pnv_phb.c +++ b/hw/pci-host/pnv_phb.c @@ -63,6 +63,7 @@ static void pnv_phb_instance_init(Object *obj) static void pnv_phb_realize(DeviceState *dev, Error **errp) { +PnvPHB *phb = PNV_PHB(dev); g_autofree char *chip_typename = pnv_phb_get_chip_typename(); g_assert(chip_typename != NULL); @@ -71,10 +72,20 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp) !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) || !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { /* PnvPHB3 */ +phb->version = PHB_VERSION_3; pnv_phb3_realize(dev, errp); return; } +if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER9)) { +phb->version = PHB_VERSION_4; +} else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER10)) { +phb->version = PHB_VERSION_5; +} else { +error_setg(errp, "unknown PNV chip: %s", chip_typename); +return; +} + pnv_phb4_realize(dev, errp); } diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h index 46158e124f..cceb37d03c 100644 --- a/include/hw/pci-host/pnv_phb.h +++ b/include/hw/pci-host/pnv_phb.h @@ -103,9 +103,14 @@ typedef struct PnvPhb4PecState PnvPhb4PecState; #define TYPE_PNV_PHB "pnv-phb" OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB, PNV_PHB) +#define PHB_VERSION_33 +#define PHB_VERSION_44 +#define PHB_VERSION_55 + struct PnvPHB { PCIExpressHost parent_obj; +uint64_t version; uint32_t chip_id; uint32_t phb_id; char bus_path[8]; @@ -142,8 +147,6 @@ struct PnvPHB { /* * PnvPHB4 attributes */ -uint64_t version; - /* The owner PEC */ PnvPhb4PecState *pec; -- 2.32.0
[PATCH 07/17] ppc/pnv: user created pnv-phb for powernv8
This patch reintroduces the powernv8 bits of the code what was removed in commit 9c10d86fee "ppc/pnv: Remove user-created PHB{3,4,5} devices", using the pnv-phb device instead of the now late pnv-phb3 device, allowing us to enable user creatable pnv-phb devices for the powernv8 machine. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb3.c | 31 +-- hw/ppc/pnv.c | 23 ++- include/hw/ppc/pnv.h | 1 + 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index d2405d4f5a..a6d6a10c52 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -993,6 +993,30 @@ void pnv_phb3_realize(DeviceState *dev, Error **errp) PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine()); int i; +/* User created devices */ +if (!phb->chip) { +Error *local_err = NULL; +BusState *s; + +phb->chip = pnv_get_chip(pnv, phb->chip_id); +if (!phb->chip) { +error_setg(errp, "invalid chip id: %d", phb->chip_id); +return; +} + +/* + * Reparent user created devices to the chip to build + * correctly the device tree. + */ +pnv_chip_parent_fixup(phb->chip, OBJECT(phb), phb->phb_id); + +s = qdev_get_parent_bus(DEVICE(phb->chip)); +if (!qdev_set_parent_bus(DEVICE(phb), s, &local_err)) { +error_propagate(errp, local_err); +return; +} +} + if (phb->phb_id >= PNV_CHIP_GET_CLASS(phb->chip)->num_phbs) { error_setg(errp, "invalid PHB index: %d", phb->phb_id); return; @@ -1052,7 +1076,10 @@ 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); +if (defaults_enabled()) { +pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), + TYPE_PNV_PHB3_ROOT_PORT); +} } void pnv_phb3_update_regions(PnvPHB *phb) @@ -1137,7 +1164,7 @@ static void pnv_phb3_root_port_class_init(ObjectClass *klass, void *data) device_class_set_parent_realize(dc, pnv_phb3_root_port_realize, &rpc->parent_realize); -dc->user_creatable = false; +dc->user_creatable = true; k->vendor_id = PCI_VENDOR_ID_IBM; k->device_id = 0x03dc; diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 12a3fe4920..d9e7530cd3 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -1152,7 +1152,9 @@ static void pnv_chip_power8_instance_init(Object *obj) object_initialize_child(obj, "homer", &chip8->homer, TYPE_PNV8_HOMER); -chip8->num_phbs = pcc->num_phbs; +if (defaults_enabled()) { +chip8->num_phbs = pcc->num_phbs; +} for (i = 0; i < chip8->num_phbs; i++) { object_initialize_child(obj, "phb[*]", &chip8->phbs[i], TYPE_PNV_PHB); @@ -1977,6 +1979,23 @@ static ICSState *pnv_ics_get(XICSFabric *xi, int irq) return NULL; } +void pnv_chip_parent_fixup(PnvChip *chip, Object *obj, int index) +{ +Object *parent = OBJECT(chip); +g_autofree char *default_id = +g_strdup_printf("%s[%d]", object_get_typename(obj), index); + +if (obj->parent == parent) { +return; +} + +object_ref(obj); +object_unparent(obj); +object_property_add_child( +parent, DEVICE(obj)->id ? DEVICE(obj)->id : default_id, obj); +object_unref(obj); +} + PnvChip *pnv_get_chip(PnvMachineState *pnv, uint32_t chip_id) { int i; @@ -2116,6 +2135,8 @@ static void pnv_machine_power8_class_init(ObjectClass *oc, void *data) pmc->compat = compat; pmc->compat_size = sizeof(compat); + +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB); } static void pnv_machine_power9_class_init(ObjectClass *oc, void *data) diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index 539454fe9d..e489bec019 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -190,6 +190,7 @@ DECLARE_INSTANCE_CHECKER(PnvChip, 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_chip_parent_fixup(PnvChip *chip, Object *obj, int index); #define TYPE_PNV_MACHINE MACHINE_TYPE_NAME("powernv") typedef struct PnvMachineClass PnvMachineClass; -- 2.32.0
[PATCH 06/17] ppc/pnv: remove PnvPHB3
The unified PnvPHB model is able to replace PnvPHB3 in all PHB3 related code. Let's do that while also removing the PnvPHB3 device entirely. The device wasn't user creatable in any QEMU release so there's no ABI concerns in doing so. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb.c | 1 + hw/pci-host/pnv_phb3.c | 92 ++ hw/pci-host/pnv_phb3_msi.c | 6 +-- hw/pci-host/pnv_phb3_pbcq.c| 8 +-- hw/ppc/pnv.c | 12 ++--- include/hw/pci-host/pnv_phb.h | 9 ++-- include/hw/pci-host/pnv_phb3.h | 43 +--- include/hw/ppc/pnv.h | 2 +- 8 files changed, 48 insertions(+), 125 deletions(-) diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c index 3dd08f768f..e4c4cca311 100644 --- a/hw/pci-host/pnv_phb.c +++ b/hw/pci-host/pnv_phb.c @@ -85,6 +85,7 @@ static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge, static Property pnv_phb_properties[] = { DEFINE_PROP_UINT32("index", PnvPHB, phb_id, 0), DEFINE_PROP_UINT32("chip-id", PnvPHB, chip_id, 0), +DEFINE_PROP_LINK("chip", PnvPHB, chip, TYPE_PNV_CHIP, PnvChip *), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index 8e31a69083..d2405d4f5a 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -24,7 +24,7 @@ qemu_log_mask(LOG_GUEST_ERROR, "phb3[%d:%d]: " fmt "\n",\ (phb)->chip_id, (phb)->phb_id, ## __VA_ARGS__) -static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb) +static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB *phb) { PCIHostState *pci = PCI_HOST_BRIDGE(phb); uint64_t addr = phb->regs3[PHB_CONFIG_ADDRESS >> 3]; @@ -43,7 +43,7 @@ static PCIDevice *pnv_phb3_find_cfg_dev(PnvPHB3 *phb) * The CONFIG_DATA register expects little endian accesses, but as the * region is big endian, we have to swap the value. */ -static void pnv_phb3_config_write(PnvPHB3 *phb, unsigned off, +static void pnv_phb3_config_write(PnvPHB *phb, unsigned off, unsigned size, uint64_t val) { uint32_t cfg_addr, limit; @@ -78,7 +78,7 @@ static void pnv_phb3_config_write(PnvPHB3 *phb, unsigned off, pci_host_config_write_common(pdev, cfg_addr, limit, val, size); } -static uint64_t pnv_phb3_config_read(PnvPHB3 *phb, unsigned off, +static uint64_t pnv_phb3_config_read(PnvPHB *phb, unsigned off, unsigned size) { uint32_t cfg_addr, limit; @@ -112,7 +112,7 @@ static uint64_t pnv_phb3_config_read(PnvPHB3 *phb, unsigned off, } } -static void pnv_phb3_check_m32(PnvPHB3 *phb) +static void pnv_phb3_check_m32(PnvPHB *phb) { uint64_t base, start, size; MemoryRegion *parent; @@ -152,7 +152,7 @@ static void pnv_phb3_check_m32(PnvPHB3 *phb) memory_region_add_subregion(parent, base, &phb->mr_m32); } -static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index) +static void pnv_phb3_check_m64(PnvPHB *phb, uint32_t index) { uint64_t base, start, size, m64; MemoryRegion *parent; @@ -202,7 +202,7 @@ static void pnv_phb3_check_m64(PnvPHB3 *phb, uint32_t index) memory_region_add_subregion(parent, base, &phb->mr_m64[index]); } -static void pnv_phb3_check_all_m64s(PnvPHB3 *phb) +static void pnv_phb3_check_all_m64s(PnvPHB *phb) { uint64_t i; @@ -211,7 +211,7 @@ static void pnv_phb3_check_all_m64s(PnvPHB3 *phb) } } -static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val) +static void pnv_phb3_lxivt_write(PnvPHB *phb, unsigned idx, uint64_t val) { uint8_t server, prio; @@ -230,7 +230,7 @@ static void pnv_phb3_lxivt_write(PnvPHB3 *phb, unsigned idx, uint64_t val) ics_write_xive(&phb->lsis, idx, server, prio, prio); } -static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, +static uint64_t *pnv_phb3_ioda_access(PnvPHB *phb, unsigned *out_table, unsigned *out_idx) { uint64_t adreg = phb->regs3[PHB_IODA_ADDR >> 3]; @@ -304,7 +304,7 @@ static uint64_t *pnv_phb3_ioda_access(PnvPHB3 *phb, return tptr; } -static uint64_t pnv_phb3_ioda_read(PnvPHB3 *phb) +static uint64_t pnv_phb3_ioda_read(PnvPHB *phb) { unsigned table; uint64_t *tptr; @@ -317,7 +317,7 @@ static uint64_t pnv_phb3_ioda_read(PnvPHB3 *phb) return *tptr; } -static void pnv_phb3_ioda_write(PnvPHB3 *phb, uint64_t val) +static void pnv_phb3_ioda_write(PnvPHB *phb, uint64_t val) { unsigned table, idx; uint64_t *tptr; @@ -345,7 +345,7 @@ static void pnv_phb3_ioda_write(PnvPHB3 *phb, uint64_t val) * This is called whenever the PHB LSI, MSI source ID register or * the PBCQ irq filters are written. */ -void pnv_phb3_remap_irqs(PnvPHB3 *phb) +void pnv_phb3_remap_irqs(PnvPHB *phb) { ICSState *ics = &phb->lsis; uint32_t local, global, count, mask, comp; @@ -408,7 +408,7 @@ void pnv_phb3_remap_irqs(PnvPHB3 *phb) pnv_phb3
[PATCH 11/17] ppc/pnv: change pnv_phb4_get_pec() to also retrieve chip10->pecs
The function assumes that we're always dealing with a PNV9_CHIP() object. This is not the case when the pnv-phb device belongs to a powernv10 machine. Change pnv_phb4_get_pec() to be able to work with PNV10_CHIP() if necessary. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb4.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index 262251ebcf..f911957f10 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -1547,17 +1547,29 @@ void pnv_phb4_instance_init(Object *obj) static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB *phb, Error **errp) { -Pnv9Chip *chip9 = PNV9_CHIP(chip); +PnvPhb4PecState *pecs = NULL; int chip_id = phb->chip_id; int index = phb->phb_id; int i, j; +if (phb->version == PHB_VERSION_4) { +Pnv9Chip *chip9 = PNV9_CHIP(chip); + +pecs = chip9->pecs; +} else if (phb->version == PHB_VERSION_5) { +Pnv10Chip *chip10 = PNV10_CHIP(chip); + +pecs = chip10->pecs; +} else { +return NULL; +} + for (i = 0; i < chip->num_pecs; i++) { /* * For each PEC, check the amount of phbs it supports * and see if the given phb4 index matches an index. */ -PnvPhb4PecState *pec = &chip9->pecs[i]; +PnvPhb4PecState *pec = &pecs[i]; for (j = 0; j < pec->num_phbs; j++) { if (index == pnv_phb4_pec_get_phb_id(pec, j)) { -- 2.32.0
[PATCH 08/17] ppc/pnv: remove PnvPHB4
The PnvPHB device is able to replace the PnvPHB4 device in all instances, while also being usable by the PHB3 logic. The PnvPHB4 device wasn't user creatable in any official QEMU release, so we can remove it without breaking ABI. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb.c | 15 hw/pci-host/pnv_phb4.c | 138 +++-- hw/pci-host/pnv_phb4_pec.c | 4 +- hw/ppc/pnv.c | 2 +- include/hw/pci-host/pnv_phb4.h | 98 +++ 5 files changed, 75 insertions(+), 182 deletions(-) diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c index e4c4cca311..9583c703d4 100644 --- a/hw/pci-host/pnv_phb.c +++ b/hw/pci-host/pnv_phb.c @@ -55,7 +55,10 @@ static void pnv_phb_instance_init(Object *obj) !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) || !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { pnv_phb3_instance_init(obj); +return; } + +pnv_phb4_instance_init(obj); } static void pnv_phb_realize(DeviceState *dev, Error **errp) @@ -69,7 +72,10 @@ static void pnv_phb_realize(DeviceState *dev, Error **errp) !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { /* PnvPHB3 */ pnv_phb3_realize(dev, errp); +return; } + +pnv_phb4_realize(dev, errp); } static const char *pnv_phb_root_bus_path(PCIHostState *host_bridge, @@ -86,6 +92,8 @@ static Property pnv_phb_properties[] = { DEFINE_PROP_UINT32("index", PnvPHB, phb_id, 0), DEFINE_PROP_UINT32("chip-id", PnvPHB, chip_id, 0), DEFINE_PROP_LINK("chip", PnvPHB, chip, TYPE_PNV_CHIP, PnvChip *), +DEFINE_PROP_LINK("pec", PnvPHB, pec, TYPE_PNV_PHB4_PEC, + PnvPhb4PecState *), DEFINE_PROP_END_OF_LIST(), }; @@ -93,12 +101,15 @@ static void pnv_phb_class_init(ObjectClass *klass, void *data) { PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); +XiveNotifierClass *xfc = XIVE_NOTIFIER_CLASS(klass); hc->root_bus_path = pnv_phb_root_bus_path; dc->realize = pnv_phb_realize; device_class_set_props(dc, pnv_phb_properties); set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); dc->user_creatable = true; + +xfc->notify = pnv_phb4_xive_notify; } static const TypeInfo pnv_phb_type_info = { @@ -107,6 +118,10 @@ static const TypeInfo pnv_phb_type_info = { .instance_size = sizeof(PnvPHB), .class_init= pnv_phb_class_init, .instance_init = pnv_phb_instance_init, +.interfaces = (InterfaceInfo[]) { +{ TYPE_XIVE_NOTIFIER }, +{ }, +}, }; static void pnv_phb_register_types(void) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index 13ba9e45d8..becfd366f1 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -47,7 +47,7 @@ static inline uint64_t SETFIELD(uint64_t mask, uint64_t word, return (word & ~mask) | ((value << ctz64(mask)) & mask); } -static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB4 *phb) +static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB *phb) { PCIHostState *pci = PCI_HOST_BRIDGE(phb); uint64_t addr = phb->regs[PHB_CONFIG_ADDRESS >> 3]; @@ -70,7 +70,7 @@ static PCIDevice *pnv_phb4_find_cfg_dev(PnvPHB4 *phb) * The CONFIG_DATA register expects little endian accesses, but as the * region is big endian, we have to swap the value. */ -static void pnv_phb4_config_write(PnvPHB4 *phb, unsigned off, +static void pnv_phb4_config_write(PnvPHB *phb, unsigned off, unsigned size, uint64_t val) { uint32_t cfg_addr, limit; @@ -105,7 +105,7 @@ static void pnv_phb4_config_write(PnvPHB4 *phb, unsigned off, pci_host_config_write_common(pdev, cfg_addr, limit, val, size); } -static uint64_t pnv_phb4_config_read(PnvPHB4 *phb, unsigned off, +static uint64_t pnv_phb4_config_read(PnvPHB *phb, unsigned off, unsigned size) { uint32_t cfg_addr, limit; @@ -142,7 +142,7 @@ static uint64_t pnv_phb4_config_read(PnvPHB4 *phb, unsigned off, /* * Root complex register accesses are memory mapped. */ -static void pnv_phb4_rc_config_write(PnvPHB4 *phb, unsigned off, +static void pnv_phb4_rc_config_write(PnvPHB *phb, unsigned off, unsigned size, uint64_t val) { PCIHostState *pci = PCI_HOST_BRIDGE(phb); @@ -163,7 +163,7 @@ static void pnv_phb4_rc_config_write(PnvPHB4 *phb, unsigned off, bswap32(val), 4); } -static uint64_t pnv_phb4_rc_config_read(PnvPHB4 *phb, unsigned off, +static uint64_t pnv_phb4_rc_config_read(PnvPHB *phb, unsigned off, unsigned size) { PCIHostState *pci = PCI_HOST_BRIDGE(phb); @@ -185,7 +185,7 @@ static uint64_t pnv_phb4_rc_config_read(PnvPHB4 *phb, unsigned off, return bswap32(val); } -static void pnv_phb4_check_mbt(PnvPHB4 *phb, uint32_t index) +stati
[PATCH 16/17] ppc/pnv: remove pnv-phb4-root-port
The unified pnv-phb-root-port can be used instead. THe pnv-phb4-root-port device isn't exposed to the user in any official QEMU release so there's no ABI breakage in removing it. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb4.c | 100 - hw/pci-host/pnv_phb4_pec.c | 4 +- include/hw/pci-host/pnv_phb4.h | 9 --- 3 files changed, 2 insertions(+), 111 deletions(-) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index fb3222d458..2dce10830d 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -1776,109 +1776,9 @@ static const TypeInfo pnv_phb4_root_bus_info = { }, }; -static void pnv_phb4_root_port_reset(DeviceState *dev) -{ -PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev); -PCIDevice *d = PCI_DEVICE(dev); -uint8_t *conf = d->config; - -rpc->parent_reset(dev); - -pci_byte_test_and_set_mask(conf + PCI_IO_BASE, - PCI_IO_RANGE_MASK & 0xff); -pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT, - PCI_IO_RANGE_MASK & 0xff); -pci_set_word(conf + PCI_MEMORY_BASE, 0); -pci_set_word(conf + PCI_MEMORY_LIMIT, 0xfff0); -pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0x1); -pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1); -pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */ -pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0x); -pci_config_set_interrupt_pin(conf, 0); -} - -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); -PnvPHB *phb = NULL; -Error *local_err = NULL; - -phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent), - TYPE_PNV_PHB); - -if (!phb) { -error_setg(errp, "%s must be connected to pnv-phb buses", dev->id); -return; -} - -/* Set unique chassis/slot values for the root port */ -qdev_prop_set_uint8(&pci->qdev, "chassis", phb->chip_id); -qdev_prop_set_uint16(&pci->qdev, "slot", phb->phb_id); - -rpc->parent_realize(dev, &local_err); -if (local_err) { -error_propagate(errp, local_err); -return; -} -} - -static void pnv_phb4_root_port_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); -PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass); - -dc->desc = "IBM PHB4 PCIE Root Port"; -dc->user_creatable = true; - -device_class_set_parent_realize(dc, pnv_phb4_root_port_realize, -&rpc->parent_realize); -device_class_set_parent_reset(dc, pnv_phb4_root_port_reset, - &rpc->parent_reset); - -k->vendor_id = PCI_VENDOR_ID_IBM; -k->device_id = PNV_PHB4_DEVICE_ID; -k->revision = 0; - -rpc->exp_offset = 0x48; -rpc->aer_offset = 0x100; - -dc->reset = &pnv_phb4_root_port_reset; -} - -static const TypeInfo pnv_phb4_root_port_info = { -.name = TYPE_PNV_PHB4_ROOT_PORT, -.parent= TYPE_PCIE_ROOT_PORT, -.instance_size = sizeof(PnvPHB4RootPort), -.class_init= pnv_phb4_root_port_class_init, -}; - -static void pnv_phb5_root_port_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - -dc->desc = "IBM PHB5 PCIE Root Port"; -dc->user_creatable = true; - -k->vendor_id = PCI_VENDOR_ID_IBM; -k->device_id = PNV_PHB5_DEVICE_ID; -} - -static const TypeInfo pnv_phb5_root_port_info = { -.name = TYPE_PNV_PHB5_ROOT_PORT, -.parent= TYPE_PNV_PHB4_ROOT_PORT, -.instance_size = sizeof(PnvPHB4RootPort), -.class_init= pnv_phb5_root_port_class_init, -}; - static void pnv_phb4_register_types(void) { type_register_static(&pnv_phb4_root_bus_info); -type_register_static(&pnv_phb5_root_port_info); -type_register_static(&pnv_phb4_root_port_info); type_register_static(&pnv_phb5_type_info); type_register_static(&pnv_phb4_iommu_memory_region_info); } diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c index 243a079ea7..51821276e9 100644 --- a/hw/pci-host/pnv_phb4_pec.c +++ b/hw/pci-host/pnv_phb4_pec.c @@ -266,7 +266,7 @@ static void pnv_pec_class_init(ObjectClass *klass, void *data) pecc->version = PNV_PHB4_VERSION; pecc->phb_type = TYPE_PNV_PHB; pecc->num_phbs = pnv_pec_num_phbs; -pecc->rp_model = TYPE_PNV_PHB4_ROOT_PORT; +pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT; } static const TypeInfo pnv_pec_type_info = { @@ -319,7 +319,7 @@ static void pnv_phb5_pec_class_init(ObjectClass *klass, void *data) pecc->version = PNV_PHB5_VERSION; pecc->phb_type = TYPE_PNV_PHB5; pecc->num_phbs = pnv_phb5_pec_num_st
[PATCH 09/17] ppc/pnv: user creatable pnv-phb for powernv9
This patch reintroduces the powernv8 bits of the code what was removed in commit 9c10d86fee "ppc/pnv: Remove user-created PHB{3,4,5} devices", using the pnv-phb device instead of the now late pnv-phb4 device, allowing us to enable user creatable pnv-phb devices for the powernv9 machine. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb4.c | 58 +- hw/pci-host/pnv_phb4_pec.c | 6 ++-- hw/ppc/pnv.c | 2 ++ 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index becfd366f1..262251ebcf 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -1544,14 +1544,70 @@ void pnv_phb4_instance_init(Object *obj) object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE); } +static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB *phb, + Error **errp) +{ +Pnv9Chip *chip9 = PNV9_CHIP(chip); +int chip_id = phb->chip_id; +int index = phb->phb_id; +int i, j; + +for (i = 0; i < chip->num_pecs; i++) { +/* + * For each PEC, check the amount of phbs it supports + * and see if the given phb4 index matches an index. + */ +PnvPhb4PecState *pec = &chip9->pecs[i]; + +for (j = 0; j < pec->num_phbs; j++) { +if (index == pnv_phb4_pec_get_phb_id(pec, j)) { +return pec; +} +} +} + +error_setg(errp, + "pnv-phb chip-id %d index %d didn't match any existing PEC", + chip_id, index); + +return NULL; +} + void pnv_phb4_realize(DeviceState *dev, Error **errp) { PnvPHB *phb = PNV_PHB(dev); +PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine()); +PnvChip *chip = pnv_get_chip(pnv, phb->chip_id); PCIHostState *pci = PCI_HOST_BRIDGE(dev); XiveSource *xsrc = &phb->xsrc; +BusState *s; +Error *local_err = NULL; int nr_irqs; char name[32]; +if (!chip) { +error_setg(errp, "invalid chip id: %d", phb->chip_id); +return; +} + +/* User created PHBs need to be assigned to a PEC */ +if (!phb->pec) { +phb->pec = pnv_phb4_get_pec(chip, phb, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} +} + +/* Reparent the PHB to the chip to build the device tree */ +pnv_chip_parent_fixup(chip, OBJECT(phb), phb->phb_id); + +s = qdev_get_parent_bus(DEVICE(chip)); +if (!qdev_set_parent_bus(DEVICE(phb), s, &local_err)) { +error_propagate(errp, local_err); +return; +} + /* Set the "big_phb" flag */ phb->big_phb = phb->phb_id == 0 || phb->phb_id == 3; @@ -1763,7 +1819,7 @@ static void pnv_phb4_root_port_class_init(ObjectClass *klass, void *data) PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass); dc->desc = "IBM PHB4 PCIE Root Port"; -dc->user_creatable = false; +dc->user_creatable = true; device_class_set_parent_realize(dc, pnv_phb4_root_port_realize, &rpc->parent_realize); diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c index 3eed560e44..243a079ea7 100644 --- a/hw/pci-host/pnv_phb4_pec.c +++ b/hw/pci-host/pnv_phb4_pec.c @@ -150,8 +150,10 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp) pec->num_phbs = pecc->num_phbs[pec->index]; /* Create PHBs if running with defaults */ -for (i = 0; i < pec->num_phbs; i++) { -pnv_pec_default_phb_realize(pec, i, errp); +if (defaults_enabled()) { +for (i = 0; i < pec->num_phbs; i++) { +pnv_pec_default_phb_realize(pec, i, errp); +} } /* Initialize the XSCOM regions for the PEC registers */ diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 34a200a29c..1a3cafcb7a 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -2155,6 +2155,8 @@ static void pnv_machine_power9_class_init(ObjectClass *oc, void *data) pmc->compat = compat; pmc->compat_size = sizeof(compat); pmc->dt_power_mgt = pnv_dt_power_mgt; + +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB); } static void pnv_machine_power10_class_init(ObjectClass *oc, void *data) -- 2.32.0
[PATCH 12/17] ppc/pnv: user creatable pnv-phb for powernv10
Given that powernv9 and powernv10 uses the same pnv-phb backend, the logic to allow user created pnv-phbs for powernv10 is already in place. This patch just flips the switch. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb4.c | 2 +- hw/ppc/pnv.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c index f911957f10..fb3222d458 100644 --- a/hw/pci-host/pnv_phb4.c +++ b/hw/pci-host/pnv_phb4.c @@ -1861,7 +1861,7 @@ static void pnv_phb5_root_port_class_init(ObjectClass *klass, void *data) PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); dc->desc = "IBM PHB5 PCIE Root Port"; -dc->user_creatable = false; +dc->user_creatable = true; k->vendor_id = PCI_VENDOR_ID_IBM; k->device_id = PNV_PHB5_DEVICE_ID; diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 1a3cafcb7a..bb193b1ed3 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -2174,6 +2174,8 @@ static void pnv_machine_power10_class_init(ObjectClass *oc, void *data) pmc->dt_power_mgt = pnv_dt_power_mgt; xfc->match_nvt = pnv10_xive_match_nvt; + +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB); } static bool pnv_machine_get_hb(Object *obj, Error **errp) -- 2.32.0
Re: [PATCH v12 2/7] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX
On Fri, May 06, 2022 at 10:57:54PM -0300, Leonardo Bras wrote: > diff --git a/io/channel-socket.c b/io/channel-socket.c > index 05c425abb8..f03a068f25 100644 > --- a/io/channel-socket.c > +++ b/io/channel-socket.c > @@ -25,9 +25,18 @@ > #include "io/channel-watch.h" > #include "trace.h" > #include "qapi/clone-visitor.h" > +#ifdef CONFIG_LINUX > +#include > +#include > + > +#if (defined(MSG_ZEROCOPY) && defined(SO_ZEROCOPY)) > +#define QEMU_MSG_ZEROCOPY > +#endif > +#endif > > #define SOCKET_MAX_FDS 16 > > + This line can be dropped when merge. > SocketAddress * > qio_channel_socket_get_local_address(QIOChannelSocket *ioc, > Error **errp) This does look nicer, imho. :) Thanks! -- Peter Xu
[PATCH 13/17] ppc/pnv: add pnv_phb_get_current_machine()
Add a simple helper to avoid hardcoding strcmp() comparisons all around pnv_phb.c. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb.c | 40 +++- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c index cef6a57d50..e03062a494 100644 --- a/hw/pci-host/pnv_phb.c +++ b/hw/pci-host/pnv_phb.c @@ -20,6 +20,10 @@ #include "sysemu/sysemu.h" +#define PNV_MACHINE_POWER81 +#define PNV_MACHINE_POWER92 +#define PNV_MACHINE_POWER10 3 + static char *pnv_phb_get_chip_typename(void) { Object *qdev_machine = qdev_get_machine(); @@ -39,7 +43,7 @@ static char *pnv_phb_get_chip_typename(void) return g_steal_pointer(&chip_typename); } -static void pnv_phb_instance_init(Object *obj) +static int pnv_phb_get_current_machine(void) { g_autofree char *chip_typename = pnv_phb_get_chip_typename(); @@ -48,12 +52,31 @@ static void pnv_phb_instance_init(Object *obj) * a valid machine->cpu_type value. */ if (!chip_typename) { -return; +return 0; } if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) || !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) || !strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { +return PNV_MACHINE_POWER8; +} else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER9)) { +return PNV_MACHINE_POWER9; +} else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER10)) { +return PNV_MACHINE_POWER10; +} + +return 0; +} + +static void pnv_phb_instance_init(Object *obj) +{ +int pnv_current_machine = pnv_phb_get_current_machine(); + +if (pnv_current_machine == 0) { +return; +} + +if (pnv_current_machine == PNV_MACHINE_POWER8) { pnv_phb3_instance_init(obj); return; } @@ -63,25 +86,24 @@ static void pnv_phb_instance_init(Object *obj) static void pnv_phb_realize(DeviceState *dev, Error **errp) { +int pnv_current_machine = pnv_phb_get_current_machine(); PnvPHB *phb = PNV_PHB(dev); -g_autofree char *chip_typename = pnv_phb_get_chip_typename(); -g_assert(chip_typename != NULL); +g_assert(pnv_current_machine != 0); -if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8) || -!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8E) || -!strcmp(chip_typename, TYPE_PNV_CHIP_POWER8NVL)) { +if (pnv_current_machine == PNV_MACHINE_POWER8) { /* PnvPHB3 */ phb->version = PHB_VERSION_3; pnv_phb3_realize(dev, errp); return; } -if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER9)) { +if (pnv_current_machine == PNV_MACHINE_POWER9) { phb->version = PHB_VERSION_4; -} else if (!strcmp(chip_typename, TYPE_PNV_CHIP_POWER10)) { +} else if (pnv_current_machine == PNV_MACHINE_POWER10) { phb->version = PHB_VERSION_5; } else { +g_autofree char *chip_typename = pnv_phb_get_chip_typename(); error_setg(errp, "unknown PNV chip: %s", chip_typename); return; } -- 2.32.0
[PATCH 14/17] ppc/pnv: add pnv-phb-root-port device
We have two very similar root-port devices, pnv-phb3-root-port and pnv-phb4-root-port. Both consist of a wrapper around the PCIESlot device that, until now, has no additional attributes. The main difference between the PHB3 and PHB4 root ports is that pnv-phb4-root-port has the pnv_phb4_root_port_reset() callback. All other differences can be merged in a single device without too much trouble. This patch introduces the unified pnv-phb-root-port that, in time, will be used as the default root port for the pnv-phb device. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb.c | 93 +++ include/hw/pci-host/pnv_phb.h | 11 + 2 files changed, 104 insertions(+) diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c index e03062a494..369dc21931 100644 --- a/hw/pci-host/pnv_phb.c +++ b/hw/pci-host/pnv_phb.c @@ -157,9 +157,102 @@ static const TypeInfo pnv_phb_type_info = { }, }; +static void pnv_phb_root_port_reset(DeviceState *dev) +{ +PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(dev); +PCIDevice *d = PCI_DEVICE(dev); +uint8_t *conf = d->config; +int pnv_current_machine = pnv_phb_get_current_machine(); + +rpc->parent_reset(dev); + +if (pnv_current_machine == PNV_MACHINE_POWER8) { +return; +} + +pci_byte_test_and_set_mask(conf + PCI_IO_BASE, + PCI_IO_RANGE_MASK & 0xff); +pci_byte_test_and_clear_mask(conf + PCI_IO_LIMIT, + PCI_IO_RANGE_MASK & 0xff); +pci_set_word(conf + PCI_MEMORY_BASE, 0); +pci_set_word(conf + PCI_MEMORY_LIMIT, 0xfff0); +pci_set_word(conf + PCI_PREF_MEMORY_BASE, 0x1); +pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, 0xfff1); +pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */ +pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0x); +pci_config_set_interrupt_pin(conf, 0); +} + +static void pnv_phb_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); +PnvPHB *phb = NULL; +Error *local_err = NULL; + +phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent), + TYPE_PNV_PHB); + +if (!phb) { +error_setg(errp, +"pnv_phb_root_port devices must be connected to pnv-phb buses"); +return; +} + +/* Set unique chassis/slot values for the root port */ +qdev_prop_set_uint8(&pci->qdev, "chassis", phb->chip_id); +qdev_prop_set_uint16(&pci->qdev, "slot", phb->phb_id); + +rpc->parent_realize(dev, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} +pci_config_set_interrupt_pin(pci->config, 0); +} + +static void pnv_phb_root_port_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass); + +dc->desc = "IBM PHB PCIE Root Port"; + +device_class_set_parent_realize(dc, pnv_phb_root_port_realize, +&rpc->parent_realize); + +device_class_set_parent_reset(dc, pnv_phb_root_port_reset, + &rpc->parent_reset); +dc->reset = &pnv_phb_root_port_reset; + +dc->user_creatable = true; + +k->vendor_id = PCI_VENDOR_ID_IBM; +/* + * k->device_id is defaulted to PNV_PHB3_DEVICE_ID. We'll fix + * it during instance_init() when we are aware of what machine + * we're running. + */ +k->device_id = 0x03dc; +k->revision = 0; + +rpc->exp_offset = 0x48; +rpc->aer_offset = 0x100; +} + +static const TypeInfo pnv_phb_root_port_info = { +.name = TYPE_PNV_PHB_ROOT_PORT, +.parent= TYPE_PCIE_ROOT_PORT, +.instance_size = sizeof(PnvPHBRootPort), +.class_init= pnv_phb_root_port_class_init, +}; + static void pnv_phb_register_types(void) { type_register_static(&pnv_phb_type_info); +type_register_static(&pnv_phb_root_port_info); } type_init(pnv_phb_register_types) diff --git a/include/hw/pci-host/pnv_phb.h b/include/hw/pci-host/pnv_phb.h index cceb37d03c..ff90a9c200 100644 --- a/include/hw/pci-host/pnv_phb.h +++ b/include/hw/pci-host/pnv_phb.h @@ -210,4 +210,15 @@ struct PnvPHB { QLIST_HEAD(, PnvPhb4DMASpace) dma_spaces; }; +/* + * PHB PCIe Root port + */ +typedef struct PnvPHBRootPort { +PCIESlot parent_obj; +} PnvPHBRootPort; + +#define TYPE_PNV_PHB_ROOT_PORT "pnv-phb-root-port" +#define PNV_PHB_ROOT_PORT(obj) \ +OBJECT_CHECK(PnvPHBRootPort, obj, TYPE_PNV_PHB_ROOT_PORT) + #endif /* PCI_HOST_PNV_PHB_H */ -- 2.32.0
[PATCH 15/17] ppc/pnv: remove pnv-phb3-root-port
The unified pnv-phb-root-port can be used in its place. There is no ABI breakage in doing so because no official QEMU release introduced user creatable pnv-phb3-root-port devices. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb3.c | 59 +- include/hw/pci-host/pnv_phb3.h | 7 2 files changed, 1 insertion(+), 65 deletions(-) diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c index a6d6a10c52..1c52df4c3f 100644 --- a/hw/pci-host/pnv_phb3.c +++ b/hw/pci-host/pnv_phb3.c @@ -1078,7 +1078,7 @@ void pnv_phb3_realize(DeviceState *dev, Error **errp) if (defaults_enabled()) { pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), - TYPE_PNV_PHB3_ROOT_PORT); + TYPE_PNV_PHB_ROOT_PORT); } } @@ -1125,66 +1125,9 @@ static const TypeInfo pnv_phb3_root_bus_info = { }, }; -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); -PnvPHB *phb = NULL; -Error *local_err = NULL; - -phb = (PnvPHB *) object_dynamic_cast(OBJECT(bus->qbus.parent), - TYPE_PNV_PHB); - -if (!phb) { -error_setg(errp, -"pnv_phb3_root_port devices must be connected to pnv-phb buses"); -return; -} - -/* Set unique chassis/slot values for the root port */ -qdev_prop_set_uint8(&pci->qdev, "chassis", phb->chip_id); -qdev_prop_set_uint16(&pci->qdev, "slot", phb->phb_id); - -rpc->parent_realize(dev, &local_err); -if (local_err) { -error_propagate(errp, local_err); -return; -} -pci_config_set_interrupt_pin(pci->config, 0); -} - -static void pnv_phb3_root_port_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); -PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass); - -dc->desc = "IBM PHB3 PCIE Root Port"; - -device_class_set_parent_realize(dc, pnv_phb3_root_port_realize, -&rpc->parent_realize); -dc->user_creatable = true; - -k->vendor_id = PCI_VENDOR_ID_IBM; -k->device_id = 0x03dc; -k->revision = 0; - -rpc->exp_offset = 0x48; -rpc->aer_offset = 0x100; -} - -static const TypeInfo pnv_phb3_root_port_info = { -.name = TYPE_PNV_PHB3_ROOT_PORT, -.parent= TYPE_PCIE_ROOT_PORT, -.instance_size = sizeof(PnvPHB3RootPort), -.class_init= pnv_phb3_root_port_class_init, -}; - static void pnv_phb3_register_types(void) { type_register_static(&pnv_phb3_root_bus_info); -type_register_static(&pnv_phb3_root_port_info); type_register_static(&pnv_phb3_iommu_memory_region_info); } diff --git a/include/hw/pci-host/pnv_phb3.h b/include/hw/pci-host/pnv_phb3.h index 1c4af98fee..417b0f99a7 100644 --- a/include/hw/pci-host/pnv_phb3.h +++ b/include/hw/pci-host/pnv_phb3.h @@ -43,13 +43,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvPBCQState, PNV_PBCQ) */ #define TYPE_PNV_PHB3_ROOT_BUS "pnv-phb3-root" -#define TYPE_PNV_PHB3_ROOT_PORT "pnv-phb3-root-port" - -typedef struct PnvPHB3RootPort { -PCIESlot parent_obj; -} PnvPHB3RootPort; - - #define PNV_PHB3_NUM_M64 16 #define PNV_PHB3_NUM_REGS (0x1000 >> 3) #define PNV_PHB3_NUM_LSI 8 -- 2.32.0
[PATCH 17/17] ppc/pnv: remove pecc->rp_model
The attribute is always being set to TYPE_PNV_PHB_ROOT_PORT. Signed-off-by: Daniel Henrique Barboza --- hw/pci-host/pnv_phb4_pec.c | 4 +--- include/hw/pci-host/pnv_phb4.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c index 51821276e9..509039bfe6 100644 --- a/hw/pci-host/pnv_phb4_pec.c +++ b/hw/pci-host/pnv_phb4_pec.c @@ -132,7 +132,7 @@ static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec, } /* Add a single Root port if running with defaults */ -pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), pecc->rp_model); +pnv_phb_attach_root_port(PCI_HOST_BRIDGE(phb), TYPE_PNV_PHB_ROOT_PORT); } static void pnv_pec_realize(DeviceState *dev, Error **errp) @@ -266,7 +266,6 @@ static void pnv_pec_class_init(ObjectClass *klass, void *data) pecc->version = PNV_PHB4_VERSION; pecc->phb_type = TYPE_PNV_PHB; pecc->num_phbs = pnv_pec_num_phbs; -pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT; } static const TypeInfo pnv_pec_type_info = { @@ -319,7 +318,6 @@ static void pnv_phb5_pec_class_init(ObjectClass *klass, void *data) pecc->version = PNV_PHB5_VERSION; pecc->phb_type = TYPE_PNV_PHB5; pecc->num_phbs = pnv_phb5_pec_num_stacks; -pecc->rp_model = TYPE_PNV_PHB_ROOT_PORT; } static const TypeInfo pnv_phb5_pec_type_info = { diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h index 8c57d836d1..b2c59ea1a0 100644 --- a/include/hw/pci-host/pnv_phb4.h +++ b/include/hw/pci-host/pnv_phb4.h @@ -116,7 +116,6 @@ struct PnvPhb4PecClass { uint64_t version; const char *phb_type; const uint32_t *num_phbs; -const char *rp_model; }; /* -- 2.32.0
Re: [PATCH v3 38/43] hw/loongarch: Add LoongArch ls7a rtc device support
On 4/29/22 05:07, Xiaojuan Yang wrote: +/* + * Shift bits and filed mask + */ +#define TOY_MON_MASK 0x3f +#define TOY_DAY_MASK 0x1f +#define TOY_HOUR_MASK 0x1f +#define TOY_MIN_MASK 0x3f +#define TOY_SEC_MASK 0x3f +#define TOY_MSEC_MASK 0xf + +#define TOY_MON_SHIFT 26 +#define TOY_DAY_SHIFT 21 +#define TOY_HOUR_SHIFT 16 +#define TOY_MIN_SHIFT 10 +#define TOY_SEC_SHIFT 4 +#define TOY_MSEC_SHIFT 0 + +#define TOY_MATCH_YEAR_MASK 0x3f +#define TOY_MATCH_MON_MASK 0xf +#define TOY_MATCH_DAY_MASK 0x1f +#define TOY_MATCH_HOUR_MASK 0x1f +#define TOY_MATCH_MIN_MASK 0x3f +#define TOY_MATCH_SEC_MASK 0x3f + +#define TOY_MATCH_YEAR_SHIFT 26 +#define TOY_MATCH_MON_SHIFT 22 +#define TOY_MATCH_DAY_SHIFT 17 +#define TOY_MATCH_HOUR_SHIFT 12 +#define TOY_MATCH_MIN_SHIFT 6 +#define TOY_MATCH_SEC_SHIFT 0 While this is ok, it would be better to use This becomes FIELD(TOY, MON, 26, 6) FIELD(TOY, DAY, 21, 5) FIELD(TOY, HOUR, 16, 5) You then extract with FIELD_EX32(val, TOY, MON), etc. I will also mention that "millisec" is misnamed in the documentation. Since it represents units of 0.1 seconds, the prefix should be "deci". +#define TOY_ENABLE_BIT (1U << 11) ... +enum { +TOYEN = 1UL << 11, +RTCEN = 1UL << 13, +}; You have two copies of the same bit. It would be best to fill in the rest of the bits in RTCCTRL, using FIELD(). +case SYS_RTCREAD0: +val = s->rtccount; +break; Surely this needs to examine qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) and scale the offset back to 32kHz. +case SYS_TOYMATCH0: +s->toymatch[0] = val; +qemu_get_timedate(&tm, s->offset); +tm.tm_sec = (val >> TOY_MATCH_SEC_SHIFT) & TOY_MATCH_SEC_MASK; +tm.tm_min = (val >> TOY_MATCH_MIN_SHIFT) & TOY_MATCH_MIN_MASK; +tm.tm_hour = ((val >> TOY_MATCH_HOUR_SHIFT) & TOY_MATCH_HOUR_MASK); +tm.tm_mday = ((val >> TOY_MATCH_DAY_SHIFT) & TOY_MATCH_DAY_MASK); +tm.tm_mon = ((val >> TOY_MATCH_MON_SHIFT) & TOY_MATCH_MON_MASK) - 1; +year_diff = ((val >> TOY_MATCH_YEAR_SHIFT) & TOY_MATCH_YEAR_MASK); +year_diff = year_diff - (tm.tm_year & TOY_MATCH_YEAR_MASK); +tm.tm_year = tm.tm_year + year_diff; +alarm_offset = qemu_timedate_diff(&tm) - s->offset; +if ((alarm_offset < 0) && (alarm_offset > -5)) { +alarm_offset = 0; +} +expire_time = qemu_clock_get_ms(rtc_clock); +expire_time += ((alarm_offset * 1000) + 100); +timer_mod(s->timer, expire_time); +break; +case SYS_TOYMATCH1: +s->toymatch[1] = val; +break; +case SYS_TOYMATCH2: +s->toymatch[2] = val; +break; Why does only register 0 affect expire time, and not all 3 registers? +case SYS_RTCCTRL: +s->cntrctl = val; +break; Need to check REN, TEN, and EO fields. +case SYS_RTCWRTIE0: +s->rtccount = val; +break; Need to compute a new rtc_offset from QEMU_CLOCK_VIRTUAL, to match RTCREAD0 above. +case SYS_RTCMATCH0: +s->rtcmatch[0] = val; +break; +case SYS_RTCMATCH1: +val = s->rtcmatch[1]; +break; +case SYS_RTCMATCH2: +val = s->rtcmatch[2]; +break; Why do these not affect expire time? +d->timer = timer_new_ms(rtc_clock, toy_timer, d); +timer_mod(d->timer, qemu_clock_get_ms(rtc_clock) + 100); Where does this number come from? In any case, after reset I would expect (1) the rtc and toy to be disabled (documentation says must initialize rtcctrl bits) and (2) I would expect all of the comparison registers to be 0 and thus no timer enabled. I guess this 100 milliseconds thing is trying to match 1 decisecond from TOYMATCH0? r~
Re: [PATCH v3 39/43] hw/loongarch: Add LoongArch load elf function.
On 4/29/22 05:07, Xiaojuan Yang wrote: Signed-off-by: Xiaojuan Yang Signed-off-by: Song Gao --- hw/loongarch/loongson3.c| 66 +++-- include/hw/loongarch/virt.h | 9 + target/loongarch/cpu.h | 2 ++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/hw/loongarch/loongson3.c b/hw/loongarch/loongson3.c index 7029d8c8b8..f9ee024f63 100644 --- a/hw/loongarch/loongson3.c +++ b/hw/loongarch/loongson3.c @@ -19,6 +19,8 @@ #include "exec/address-spaces.h" #include "hw/irq.h" #include "net/net.h" +#include "hw/loader.h" +#include "elf.h" #include "hw/intc/loongarch_ipi.h" #include "hw/intc/loongarch_extioi.h" #include "hw/intc/loongarch_pch_pic.h" @@ -29,6 +31,36 @@ #include "target/loongarch/cpu.h" +static struct _loaderparams { +unsigned long ram_size; +const char *kernel_filename; +} loaderparams; Never use "unsigned long", only "uint{32,64}_t" or "size_t". Otherwise you don't know what the host is going to give you. +static int64_t load_kernel_info(void) +{ +int64_t kernel_entry, kernel_low, kernel_high; Why are you using signed values here, +long kernel_size; + +kernel_size = load_elf(loaderparams.kernel_filename, NULL, + cpu_loongarch_virt_to_phys, NULL, + (uint64_t *)&kernel_entry, (uint64_t *)&kernel_low, + (uint64_t *)&kernel_high, NULL, 0, and casting them? Oh, and kernel_size must be ssize_t. @@ -237,22 +283,38 @@ static void loongarch_init(MachineState *machine) cpu_create(machine->cpu_type); } +if (ram_size < 1 * GiB) { +error_report("ram_size must be greater than 1G."); +exit(1); +} Why is this here? It's certainly not related to load_elf. r~
Re: [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers
On 04.05.22 00:51, Ilya Leoshkevich wrote: > Commit 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp") > removed an unused field from rt_sigframe, disturbing offsets of other > fields and breaking unwinding from signal handlers (e.g. libgcc's > s390_fallback_frame() relies on this struct having a specific layout). > Restore the field and add a comment. > > Reported-by: Ulrich Weigand > Signed-off-by: Ilya Leoshkevich > Fixes: 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp") > --- > linux-user/s390x/signal.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c > index f47713e04a..4979c4b017 100644 > --- a/linux-user/s390x/signal.c > +++ b/linux-user/s390x/signal.c > @@ -84,6 +84,11 @@ struct target_ucontext { > > typedef struct { > uint8_t callee_used_stack[__SIGNAL_FRAMESIZE]; > +/* > + * This field is no longer initialized by the kernel, but it's still a > part > + * of the ABI. > + */ > +uint16_t svc_insn; > struct target_siginfo info; > struct target_ucontext uc; > } rt_sigframe; Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH 1/2] linux-user/s390x: Fix unwinding from signal handlers
On 5/3/22 17:51, Ilya Leoshkevich wrote: Commit 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp") removed an unused field from rt_sigframe, disturbing offsets of other fields and breaking unwinding from signal handlers (e.g. libgcc's s390_fallback_frame() relies on this struct having a specific layout). Restore the field and add a comment. Reported-by: Ulrich Weigand Signed-off-by: Ilya Leoshkevich Fixes: 31330e6cecfd ("linux-user/s390x: Implement setup_sigtramp") --- linux-user/s390x/signal.c | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 30/50] lasi: use qdev GPIOs to wire up IRQs in lasi_initfn()
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c | 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 31/50] lasi: fix serial port initialisation
On 5/4/22 04:25, Mark Cave-Ayland wrote: The existing code checks for serial_hd(1) but sets the LASI serial port chardev to serial_hd(0). Use serial_hd(1) for the LASI serial port and also set the serial port endian to DEVICE_BIG_ENDIAN (which also matches the endian of the existing serial port). Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 31/50] lasi: fix serial port initialisation
On 5/4/22 04:25, Mark Cave-Ayland wrote: The existing code checks for serial_hd(1) but sets the LASI serial port chardev to serial_hd(0). Use serial_hd(1) for the LASI serial port and also set the serial port endian to DEVICE_BIG_ENDIAN (which also matches the endian of the existing serial port). Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 32/50] lasi: update lasi_initfn() to return LASIState
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 3 ++- hw/hppa/lasi.c | 4 ++-- hw/hppa/machine.c | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 33/50] lasi: move LAN initialisation to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c| 7 --- hw/hppa/machine.c | 5 + 2 files changed, 5 insertions(+), 7 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 34/50] lasi: move parallel port initialisation to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c| 6 -- hw/hppa/machine.c | 6 ++ 2 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 35/50] lasi: move second serial port initialisation to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c| 8 hw/hppa/machine.c | 7 +++ 2 files changed, 7 insertions(+), 8 deletions(-) In that it is code movement, Reviewed-by: Richard Henderson +if (serial_hd(1)) { +/* Serial port */ +serial_mm_init(addr_space, LASI_UART_HPA + 0x800, 0, +qdev_get_gpio_in(lasi_dev, LASI_IRQ_UART_HPA), 800 / 16, +serial_hd(1), DEVICE_BIG_ENDIAN); +} Although I believe there's a bug: both of these serial ports are being registered at LASI_UART_HPA + 0x800. I suspect this bug was hidden by the bug fixed in patch 31, in that serial_hd(0) should be at 0x800, and serial_hd(1) should be elsewhere. r~
Re: [PATCH v2 36/50] lasi: move PS2 initialisation to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c| 5 - hw/hppa/machine.c | 5 + 2 files changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 37/50] lasi: remove address space parameter from lasi_initfn()
On 5/4/22 04:25, Mark Cave-Ayland wrote: Now that all of the LASI devices are mapped by the board, this parameter is no longer required. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 2 +- hw/hppa/lasi.c | 2 +- hw/hppa/machine.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 38/50] lasi: move lasi_initfn() to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: Move the simplified lasi_initfn() back to machine.c whilst also renaming it back to its original lasi_init() name. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 2 -- hw/hppa/lasi.c | 10 -- hw/hppa/machine.c | 12 +++- 3 files changed, 11 insertions(+), 13 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 40/50] lasi: use numerical constant for iar reset value
On 5/4/22 04:25, Mark Cave-Ayland wrote: This is to allow us to decouple the LASI device from the board logic. If it is decided later that this value needs to be configurable then it can easily be converted to a qdev property. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Richard Henderson r~
Re: [PATCH v2 41/50] hppa: move device headers from hppa_sys.h into individual .c files
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 3 --- hw/hppa/lasi.h | 4 hw/hppa/machine.c | 2 ++ hw/hppa/pci.c | 3 +++ 4 files changed, 9 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 39/50] lasi: use constants for device register offsets
On 5/4/22 04:25, Mark Cave-Ayland wrote: Instead of generating the offset based upon the physical address of the register, add constants for each of the device registers to lasi.h and update lasi.c to use them. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/lasi.c | 28 ++-- hw/hppa/lasi.h | 5 + 2 files changed, 19 insertions(+), 14 deletions(-) Worth removing the final usages, and the old definitions in hppa_machine.h? Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH v2 42/50] lasi: move from hw/hppa to hw/misc
On 5/4/22 04:25, Mark Cave-Ayland wrote: Move the LASI device implementation from hw/hppa to hw/misc so that it is located with all the other miscellaneous devices. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- MAINTAINERS | 2 ++ hw/hppa/Kconfig | 1 + hw/hppa/machine.c | 2 +- hw/hppa/meson.build | 2 +- hw/hppa/trace-events| 5 - hw/misc/Kconfig | 3 +++ hw/{hppa => misc}/lasi.c| 3 +-- hw/misc/meson.build | 3 +++ hw/misc/trace-events| 5 + {hw/hppa => include/hw/misc}/lasi.h | 0 10 files changed, 17 insertions(+), 9 deletions(-) rename hw/{hppa => misc}/lasi.c (99%) rename {hw/hppa => include/hw/misc}/lasi.h (100%) I don't understand hw/misc, or why this is a better categorization than hw/hppa. r~
Re: [PATCH v2 43/50] hppa: move hppa_pci_ignore_ops from pci.c to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: The memory region only has one user which is for ensuring accesses to the ISA bus memory do not fault. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 1 - hw/hppa/machine.c | 23 +++ hw/hppa/pci.c | 26 -- 3 files changed, 23 insertions(+), 27 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 44/50] hppa: remove hw/hppa/pci.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: The functions and definitions in this file are not used anywhere within the generic hppa machine. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 6 hw/hppa/meson.build | 2 +- hw/hppa/pci.c| 65 hw/hppa/trace-events | 3 -- 4 files changed, 1 insertion(+), 75 deletions(-) delete mode 100644 hw/hppa/pci.c Reviewed-by: Richard Henderson r~
Re: [PATCH v2 45/50] hppa: remove unused trace-events from from hw/hppa
On 5/4/22 04:25, Mark Cave-Ayland wrote: Now that there are no longer any devices in hw/hppa the trace-events file is empty and can be removed. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/trace-events | 1 - meson.build | 1 - 2 files changed, 2 deletions(-) delete mode 100644 hw/hppa/trace-events Reviewed-by: Richard Henderson r~
Re: [PATCH v2 46/50] hppa: move enable_lan() define from hppa_sys.h to machine.c
On 5/4/22 04:25, Mark Cave-Ayland wrote: Now that the board configuration is in one place, the define is only needed when wiring up the board in machine.c. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 2 -- hw/hppa/machine.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 47/50] hppa: remove the empty hppa_sys.h file
On 5/4/22 04:25, Mark Cave-Ayland wrote: This file is now just a simple wrapper that includes hppa_hardware.h so remove the file completely, and update its single user in machine.c to include hppa_hardware.h directly. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/hppa_sys.h | 8 hw/hppa/machine.c | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) delete mode 100644 hw/hppa/hppa_sys.h Reviewed-by: Richard Henderson r~
Re: [PATCH v2 48/50] hppa: use MACHINE QOM macros for defining the hppa machine
On 5/4/22 04:25, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/machine.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 50/50] hppa: simplify machine function names in machine.c
On 5/4/22 04:26, Mark Cave-Ayland wrote: Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/machine.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 49/50] hppa: fold machine_hppa_machine_init() into machine_hppa_machine_init_class_init()
On 5/4/22 04:25, Mark Cave-Ayland wrote: There is no need for a separate function to set the machine class properties separately from the others. Signed-off-by: Mark Cave-Ayland Acked-by: Helge Deller --- hw/hppa/machine.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 1/7] target/ppc: Implement xxm[tf]acc and xxsetaccz
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: From: "Lucas Mateus Castro (alqotel)" Implement the following PowerISA v3.1 instructions: xxmfacc: VSX Move From Accumulator xxmtacc: VSX Move To Accumulator xxsetaccz: VSX Set Accumulator to Zero The PowerISA 3.1 mentions that for the current version of the architecture, "the hardware implementation provides the effect of ACC[i] and VSRs 4*i to 4*i + 3 logically containing the same data" and "The Accumulators introduce no new logical state at this time" (page 501). For now it seems unnecessary to create new structures, so this patch just uses ACC[i] as VSRs 4*i to 4*i+3 and therefore move to and from accumulators are no-ops. Signed-off-by: Lucas Mateus Castro (alqotel) --- target/ppc/cpu.h| 5 + target/ppc/insn32.decode| 9 + target/ppc/translate/vsx-impl.c.inc | 31 + 3 files changed, 45 insertions(+) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 2/7] target/ppc: Implemented xvi*ger* instructions
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 10c6d7ae43..348a898950 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -238,6 +238,7 @@ typedef union _ppc_vsr_t { typedef ppc_vsr_t ppc_avr_t; typedef ppc_vsr_t ppc_fprp_t; +typedef ppc_vsr_t ppc_acc_t; #if !defined(CONFIG_USER_ONLY) /* Software TLB cache */ diff --git a/target/ppc/helper.h b/target/ppc/helper.h index aa6773c4a5..61217e0a10 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -537,6 +537,15 @@ DEF_HELPER_5(XXBLENDVB, void, vsr, vsr, vsr, vsr, i32) DEF_HELPER_5(XXBLENDVH, void, vsr, vsr, vsr, vsr, i32) DEF_HELPER_5(XXBLENDVW, void, vsr, vsr, vsr, vsr, i32) DEF_HELPER_5(XXBLENDVD, void, vsr, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI4GER8, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI4GER8PP, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI8GER4, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI8GER4PP, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI8GER4SPP, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI16GER2, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI16GER2S, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI16GER2PP, void, env, vsr, vsr, vsr, i32) +DEF_HELPER_5(XVI16GER2SPP, void, env, vsr, vsr, vsr, i32) Did you intend to use "acc" here, for documentation purposes? It's just a couple of #defines above. Either way, much cleaner. Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 3/7] target/ppc: Implemented pmxvi*ger* instructions
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: return true; + +} +static bool do_ger_XX3(DisasContext *ctx, arg_XX3 *a, Watch the whitespace. +{ +arg_MMIRR_XX3 m; +m.xa = a->xa; +m.xb = a->xb; +m.xt = a->xt; +m.pmsk = 0xFF; +m.ymsk = 0xF; +m.xmsk = 0xF; +return do_ger_MMIRR_XX3(ctx, &m, helper); } Is XX3 going to be used for anything else? Is it worthwhile to move these into the decoder as explicit assignments? Either way, Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 4/7] target/ppc: Implemented xvf*ger*
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: From: "Lucas Mateus Castro (alqotel)" Implement the following PowerISA v3.1 instructions: xvf32ger: VSX Vector 32-bit Floating-Point GER (rank-1 update) xvf32gernn: VSX Vector 32-bit Floating-Point GER (rank-1 update) Negative multiply, Negative accumulate xvf32gernp: VSX Vector 32-bit Floating-Point GER (rank-1 update) Negative multiply, Positive accumulate xvf32gerpn: VSX Vector 32-bit Floating-Point GER (rank-1 update) Positive multiply, Negative accumulate xvf32gerpp: VSX Vector 32-bit Floating-Point GER (rank-1 update) Positive multiply, Positive accumulate xvf64ger: VSX Vector 64-bit Floating-Point GER (rank-1 update) xvf64gernn: VSX Vector 64-bit Floating-Point GER (rank-1 update) Negative multiply, Negative accumulate xvf64gernp: VSX Vector 64-bit Floating-Point GER (rank-1 update) Negative multiply, Positive accumulate xvf64gerpn: VSX Vector 64-bit Floating-Point GER (rank-1 update) Positive multiply, Negative accumulate xvf64gerpp: VSX Vector 64-bit Floating-Point GER (rank-1 update) Positive multiply, Positive accumulate Signed-off-by: Lucas Mateus Castro (alqotel) --- target/ppc/cpu.h| 4 + target/ppc/fpu_helper.c | 178 target/ppc/helper.h | 10 ++ target/ppc/insn32.decode| 13 ++ target/ppc/translate/vsx-impl.c.inc | 12 ++ 5 files changed, 217 insertions(+) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 348a898950..eb50ad699e 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -2639,6 +2639,8 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx) #define VsrSW(i) s32[i] #define VsrD(i) u64[i] #define VsrSD(i) s64[i] +#define VsrSF(i) f32[i] +#define VsrDF(i) f64[i] #else #define VsrB(i) u8[15 - (i)] #define VsrSB(i) s8[15 - (i)] @@ -2648,6 +2650,8 @@ static inline bool lsw_reg_in_range(int start, int nregs, int rx) #define VsrSW(i) s32[3 - (i)] #define VsrD(i) u64[1 - (i)] #define VsrSD(i) s64[1 - (i)] +#define VsrSF(i) f32[3 - (i)] +#define VsrDF(i) f64[1 - (i)] #endif static inline int vsr64_offset(int i, bool high) diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index f6c8318a71..138b30d08f 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -3462,3 +3462,181 @@ void helper_xssubqp(CPUPPCState *env, uint32_t opcode, *xt = t; do_float_check_status(env, GETPC()); } + +static void set_rounding_mode_rn(CPUPPCState *env) +{ +uint8_t rmode = (env->fpscr & FP_RN) >> FPSCR_RN0; +switch (rmode) { +case 0: +set_float_rounding_mode(float_round_nearest_even, &env->fp_status); +break; +case 1: +set_float_rounding_mode(float_round_to_zero, &env->fp_status); +break; +case 2: +set_float_rounding_mode(float_round_up, &env->fp_status); +break; +case 3: +set_float_rounding_mode(float_round_down, &env->fp_status); +break; +default: +abort(); +} +} How is this different from fpscr_set_rounding_mode and why do you need to call it at all? r~
Re: [RFC PATCH v2 6/7] target/ppc: Implemented pmxvf*ger*
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: From: "Lucas Mateus Castro (alqotel)" Implement the following PowerISA v3.1 instructions: pmxvf16ger2: Prefixed Masked VSX Vector 16-bit Floating-Point GER (rank-2 update) pmxvf16ger2nn: Prefixed Masked VSX Vector 16-bit Floating-Point GER (rank-2 update) Negative multiply, Negative accumulate pmxvf16ger2np: Prefixed Masked VSX Vector 16-bit Floating-Point GER (rank-2 update) Negative multiply, Positive accumulate pmxvf16ger2pn: Prefixed Masked VSX Vector 16-bit Floating-Point GER (rank-2 update) Positive multiply, Negative accumulate pmxvf16ger2pp: Prefixed Masked VSX Vector 16-bit Floating-Point GER (rank-2 update) Positive multiply, Positive accumulate pmxvf32ger:Prefixed Masked VSX Vector 32-bit Floating-Point GER (rank-1 update) pmxvf32gernn: Prefixed Masked VSX Vector 32-bit Floating-Point GER (rank-1 update) Negative multiply, Negative accumulate pmxvf32gernp: Prefixed Masked VSX Vector 32-bit Floating-Point GER (rank-1 update) Negative multiply, Positive accumulate pmxvf32gerpn: Prefixed Masked VSX Vector 32-bit Floating-Point GER (rank-1 update) Positive multiply, Negative accumulate pmxvf32gerpp: Prefixed Masked VSX Vector 32-bit Floating-Point GER (rank-1 update) Positive multiply, Positive accumulate pmxvf64ger:Prefixed Masked VSX Vector 64-bit Floating-Point GER (rank-1 update) pmxvf64gernn: Prefixed Masked VSX Vector 64-bit Floating-Point GER (rank-1 update) Negative multiply, Negative accumulate pmxvf64gernp: Prefixed Masked VSX Vector 64-bit Floating-Point GER (rank-1 update) Negative multiply, Positive accumulate pmxvf64gerpn: Prefixed Masked VSX Vector 64-bit Floating-Point GER (rank-1 update) Positive multiply, Negative accumulate pmxvf64gerpp: Prefixed Masked VSX Vector 64-bit Floating-Point GER (rank-1 update) Positive multiply, Positive accumulate Signed-off-by: Lucas Mateus Castro (alqotel) --- target/ppc/insn64.decode| 38 + target/ppc/translate/vsx-impl.c.inc | 18 ++ 2 files changed, 56 insertions(+) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v2 5/7] target/ppc: Implemented xvf16ger*
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: +static inline float32 float32_neg(float32 a) +{ +if (((a & 0x7f80) == 0x7f80) && (a & 0x007f)) { +return a; +} else { +return float32_chs(a); +} +} This is wrong -- even NaNs get their signs changed. Negation and absolute value are non-arithmetic operations. If you're matching hardware results, this suggests... +if (neg_mul) { +msum = float32_neg(msum); +} +if (neg_acc) { +aux_acc = float32_neg(at[i].VsrSF(j)); +} else { +aux_acc = at[i].VsrSF(j); +} +at[i].VsrSF(j) = float32_add(msum, aux_acc, excp_ptr); This "add" should be "sub" instead of using a separate negation, when required. I do wonder about the double-negation vs nans. It looks like this could be float32_muladd(float32_one, msum, aux_acc, flags, &status) with flags set to float_muladd_negate_* for neg_mul and neg_acc. Any NaNs would go through pick_nan_muladd and fail to be altered. I'm not sure if I'm suggesting actual use of muladd, for the simplicity, or if you should have an inline check for nans. I might need to think about this in the morning. r~
Re: [RFC PATCH v2 7/7] target/ppc: Implemented [pm]xvbf16ger2*
On 5/6/22 07:18, Lucas Mateus Castro(alqotel) wrote: From: "Lucas Mateus Castro (alqotel)" Implement the following PowerISA v3.1 instructions: xvbf16ger2: VSX Vector bfloat16 GER (rank-2 update) xvbf16ger2nn: VSX Vector bfloat16 GER (rank-2 update) Negative multiply, Negative accumulate xvbf16ger2np: VSX Vector bfloat16 GER (rank-2 update) Negative multiply, Positive accumulate xvbf16ger2pn: VSX Vector bfloat16 GER (rank-2 update) Positive multiply, Negative accumulate xvbf16ger2pp: VSX Vector bfloat16 GER (rank-2 update) Positive multiply, Positive accumulate pmxvbf16ger2: Prefixed Masked VSX Vector bfloat16 GER (rank-2 update) pmxvbf16ger2nn: Prefixed Masked VSX Vector bfloat16 GER (rank-2 update) Negative multiply, Negative accumulate pmxvbf16ger2np: Prefixed Masked VSX Vector bfloat16 GER (rank-2 update) Negative multiply, Positive accumulate pmxvbf16ger2pn: Prefixed Masked VSX Vector bfloat16 GER (rank-2 update) Positive multiply, Negative accumulate pmxvbf16ger2pp: Prefixed Masked VSX Vector bfloat16 GER (rank-2 update) Positive multiply, Positive accumulate Signed-off-by: Lucas Mateus Castro (alqotel) --- There's a discrepancy between this implementation and mambo/the hardware where implementing it with float64_mul then float64r32_muladd sometimes results in an incorrect result after an underflow, but implementing with float32_mul then float32_muladd results in incorrect signal in some 0 or infinite results. I've not been able to solve this I did suggest that the float64_mul needs to be done in round-to-odd. Anyway, for this patch, Reviewed-by: Richard Henderson r~