1 | A last lot of bug fixes before rc0... | 1 | A last small test of bug fixes before rc1. |
---|---|---|---|
2 | 2 | ||
3 | thanks | 3 | thanks |
4 | -- PMM | 4 | -- PMM |
5 | 5 | ||
6 | The following changes since commit 0d0275c31f00b71b49eb80bbdca2cfe244cf80fb: | 6 | The following changes since commit ed8ad9728a9c0eec34db9dff61dfa2f1dd625637: |
7 | 7 | ||
8 | Merge tag 'net-pull-request' of https://github.com/jasowang/qemu into staging (2022-07-26 10:31:02 +0100) | 8 | Merge tag 'pull-tpm-2023-07-14-1' of https://github.com/stefanberger/qemu-tpm into staging (2023-07-15 14:54:04 +0100) |
9 | 9 | ||
10 | are available in the Git repository at: | 10 | are available in the Git repository at: |
11 | 11 | ||
12 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220726 | 12 | https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230717 |
13 | 13 | ||
14 | for you to fetch changes up to 5865d99fe88d8c8fa437c18c6b63fb2a8165634f: | 14 | for you to fetch changes up to c2c1c4a35c7c2b1a4140b0942b9797c857e476a4: |
15 | 15 | ||
16 | hw/display/bcm2835_fb: Fix framebuffer allocation address (2022-07-26 14:09:44 +0100) | 16 | hw/nvram: Avoid unnecessary Xilinx eFuse backstore write (2023-07-17 11:05:52 +0100) |
17 | 17 | ||
18 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
19 | target-arm queue: | 19 | target-arm queue: |
20 | * Update Coverity component definitions | 20 | * hw/arm/sbsa-ref: set 'slots' property of xhci |
21 | * target/arm: Add MO_128 entry to pred_esz_masks[] | 21 | * linux-user: Remove pointless NULL check in clock_adjtime handling |
22 | * configure: Fix portability issues | 22 | * ptw: Fix S1_ptw_translate() debug path |
23 | * hw/display/bcm2835_fb: Fix framebuffer allocation address | 23 | * ptw: Account for FEAT_RME when applying {N}SW, SA bits |
24 | * accel/tcg: Zero-pad PC in TCG CPU exec trace lines | ||
25 | * hw/nvram: Avoid unnecessary Xilinx eFuse backstore write | ||
24 | 26 | ||
25 | ---------------------------------------------------------------- | 27 | ---------------------------------------------------------------- |
26 | Alan Jian (1): | 28 | Peter Maydell (5): |
27 | hw/display/bcm2835_fb: Fix framebuffer allocation address | 29 | linux-user: Remove pointless NULL check in clock_adjtime handling |
30 | target/arm/ptw.c: Add comments to S1Translate struct fields | ||
31 | target/arm: Fix S1_ptw_translate() debug path | ||
32 | target/arm/ptw.c: Account for FEAT_RME when applying {N}SW, SA bits | ||
33 | accel/tcg: Zero-pad PC in TCG CPU exec trace lines | ||
28 | 34 | ||
29 | Peter Maydell (8): | 35 | Tong Ho (1): |
30 | scripts/coverity-scan/COMPONENTS.md: Add loongarch component | 36 | hw/nvram: Avoid unnecessary Xilinx eFuse backstore write |
31 | scripts/coverity-scan/COMPONENTS.md: Update slirp component info | ||
32 | target/arm: Add MO_128 entry to pred_esz_masks[] | ||
33 | configure: Add missing POSIX-required space | ||
34 | configure: Add braces to clarify intent of $emu[[:space:]] | ||
35 | configure: Don't use bash-specific string-replacement syntax | ||
36 | configure: Drop dead code attempting to use -msmall-data on alpha hosts | ||
37 | configure: Avoid '==' bashism | ||
38 | 37 | ||
39 | configure | 20 +++++++------------- | 38 | Yuquan Wang (1): |
40 | target/arm/cpu.h | 2 +- | 39 | hw/arm/sbsa-ref: set 'slots' property of xhci |
41 | hw/display/bcm2835_fb.c | 3 +-- | 40 | |
42 | target/arm/translate-sve.c | 5 +++-- | 41 | accel/tcg/cpu-exec.c | 4 +-- |
43 | scripts/coverity-scan/COMPONENTS.md | 7 +++++-- | 42 | accel/tcg/translate-all.c | 2 +- |
44 | 5 files changed, 17 insertions(+), 20 deletions(-) | 43 | hw/arm/sbsa-ref.c | 1 + |
44 | hw/nvram/xlnx-efuse.c | 11 ++++-- | ||
45 | linux-user/syscall.c | 12 +++---- | ||
46 | target/arm/ptw.c | 90 +++++++++++++++++++++++++++++++++++++++++------ | ||
47 | 6 files changed, 98 insertions(+), 22 deletions(-) | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | Add the component regex for the new loongarch target. | ||
2 | 1 | ||
3 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
4 | Acked-by: Paolo Bonzini <pbonzini@redhat.com> | ||
5 | Message-id: 20220718142310.16013-2-peter.maydell@linaro.org | ||
6 | --- | ||
7 | scripts/coverity-scan/COMPONENTS.md | 3 +++ | ||
8 | 1 file changed, 3 insertions(+) | ||
9 | |||
10 | diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md | ||
11 | index XXXXXXX..XXXXXXX 100644 | ||
12 | --- a/scripts/coverity-scan/COMPONENTS.md | ||
13 | +++ b/scripts/coverity-scan/COMPONENTS.md | ||
14 | @@ -XXX,XX +XXX,XX @@ testlibs | ||
15 | |||
16 | tests | ||
17 | ~ (/qemu)?(/tests/.*) | ||
18 | + | ||
19 | +loongarch | ||
20 | + ~ (/qemu)?((/include)?/hw/(loongarch/.*|.*/loongarch.*)|/target/loongarch/.*) | ||
21 | -- | ||
22 | 2.25.1 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | Update the regex for the slirp component now that it lives | ||
2 | solely inside /slirp/, and note that it should be ignored in | ||
3 | Coverity analysis (because it's a separate upstream project | ||
4 | now, and they run Coverity on it themselves). | ||
5 | 1 | ||
6 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
7 | Acked-by: Paolo Bonzini <pbonzini@redhat.com> | ||
8 | Message-id: 20220718142310.16013-3-peter.maydell@linaro.org | ||
9 | --- | ||
10 | scripts/coverity-scan/COMPONENTS.md | 4 ++-- | ||
11 | 1 file changed, 2 insertions(+), 2 deletions(-) | ||
12 | |||
13 | diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/scripts/coverity-scan/COMPONENTS.md | ||
16 | +++ b/scripts/coverity-scan/COMPONENTS.md | ||
17 | @@ -XXX,XX +XXX,XX @@ qemu-ga | ||
18 | scsi | ||
19 | ~ (/qemu)?(/scsi/.*|/hw/scsi/.*|/include/hw/scsi/.*) | ||
20 | |||
21 | -slirp | ||
22 | - ~ (/qemu)?(/.*slirp.*) | ||
23 | +slirp (component should be ignored in analysis) | ||
24 | + ~ (/qemu)?(/slirp/.*) | ||
25 | |||
26 | tcg | ||
27 | ~ (/qemu)?(/accel/tcg/.*|/replay/.*|/(.*/)?softmmu.*) | ||
28 | -- | ||
29 | 2.25.1 | diff view generated by jsdifflib |
1 | From: Alan Jian <alanjian85@gmail.com> | 1 | From: Yuquan Wang <wangyuquan1236@phytium.com.cn> |
---|---|---|---|
2 | 2 | ||
3 | This patch fixes the dedicated framebuffer mailbox interface by | 3 | This extends the slots of xhci to 64, since the default xhci_sysbus |
4 | removing an unneeded offset. This means that we pick the framebuffer | 4 | just supports one slot. |
5 | address in the same way that we do if the guest code uses the buffer | ||
6 | allocate mechanism of the bcm2835_property interface (case | ||
7 | 0x00040001: /* Allocate buffer */ in bcm2835_property.c). | ||
8 | 5 | ||
9 | The documentation of this mailbox interface doesn't say anything | 6 | Signed-off-by: Wang Yuquan <wangyuquan1236@phytium.com.cn> |
10 | about using parts of the request buffer address to affect the | 7 | Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> |
11 | chosen framebuffer address: | 8 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
12 | https://github.com/raspberrypi/firmware/wiki/Mailbox-framebuffer-interface | 9 | Reviewed-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> |
13 | 10 | Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org> | |
14 | Some baremetal applications like the Screen01/Screen02 examples from | 11 | Message-id: 20230710063750.473510-2-wangyuquan1236@phytium.com.cn |
15 | Baking Pi tutorial[1] didn't work before this patch. | ||
16 | |||
17 | [1] https://www.cl.cam.ac.uk/projects/raspberrypi/tutorials/os/screen01.html | ||
18 | |||
19 | Signed-off-by: Alan Jian <alanjian85@outlook.com> | ||
20 | Message-id: 20220725145838.8412-1-alanjian85@outlook.com | ||
21 | [PMM: tweaked commit message] | ||
22 | Reviewed-by: Peter Maydell <peter.maydell@linaro.org> | ||
23 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
24 | --- | 13 | --- |
25 | hw/display/bcm2835_fb.c | 3 +-- | 14 | hw/arm/sbsa-ref.c | 1 + |
26 | 1 file changed, 1 insertion(+), 2 deletions(-) | 15 | 1 file changed, 1 insertion(+) |
27 | 16 | ||
28 | diff --git a/hw/display/bcm2835_fb.c b/hw/display/bcm2835_fb.c | 17 | diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c |
29 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
30 | --- a/hw/display/bcm2835_fb.c | 19 | --- a/hw/arm/sbsa-ref.c |
31 | +++ b/hw/display/bcm2835_fb.c | 20 | +++ b/hw/arm/sbsa-ref.c |
32 | @@ -XXX,XX +XXX,XX @@ static void bcm2835_fb_mbox_push(BCM2835FBState *s, uint32_t value) | 21 | @@ -XXX,XX +XXX,XX @@ static void create_xhci(const SBSAMachineState *sms) |
33 | newconf.xoffset = ldl_le_phys(&s->dma_as, value + 24); | 22 | hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base; |
34 | newconf.yoffset = ldl_le_phys(&s->dma_as, value + 28); | 23 | int irq = sbsa_ref_irqmap[SBSA_XHCI]; |
35 | 24 | DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS); | |
36 | - newconf.base = s->vcram_base | (value & 0xc0000000); | 25 | + qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS); |
37 | - newconf.base += BCM2835_FB_OFFSET; | 26 | |
38 | + newconf.base = s->vcram_base + BCM2835_FB_OFFSET; | 27 | sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); |
39 | 28 | sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); | |
40 | /* Copy fields which we don't want to change from the existing config */ | ||
41 | newconf.pixo = s->config.pixo; | ||
42 | -- | 29 | -- |
43 | 2.25.1 | 30 | 2.34.1 | diff view generated by jsdifflib |
1 | The '==' operator to test is a bashism; the standard way to copmare | 1 | In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to |
---|---|---|---|
2 | strings is '='. This causes dash to complain: | 2 | the address of the local variable htx. This means it can never be |
3 | NULL, but later in the code we check it for NULL anyway. Coverity | ||
4 | complains about this (CID 1507683) because the NULL check comes after | ||
5 | a call to clock_adjtime() that assumes it is non-NULL. | ||
3 | 6 | ||
4 | ../../configure: 681: test: linux: unexpected operator | 7 | Since phtx is always &htx, and is used only in three places, it's not |
8 | really necessary. Remove it, bringing the code structure in to line | ||
9 | with that for TARGET_NR_clock_adjtime64, which already uses a simple | ||
10 | '&htx' when it wants a pointer to 'htx'. | ||
5 | 11 | ||
6 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
7 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 13 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
8 | Message-id: 20220720152631.450903-6-peter.maydell@linaro.org | 14 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
15 | Message-id: 20230623144410.1837261-1-peter.maydell@linaro.org | ||
9 | --- | 16 | --- |
10 | configure | 2 +- | 17 | linux-user/syscall.c | 12 +++++------- |
11 | 1 file changed, 1 insertion(+), 1 deletion(-) | 18 | 1 file changed, 5 insertions(+), 7 deletions(-) |
12 | 19 | ||
13 | diff --git a/configure b/configure | 20 | diff --git a/linux-user/syscall.c b/linux-user/syscall.c |
14 | index XXXXXXX..XXXXXXX 100755 | 21 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/configure | 22 | --- a/linux-user/syscall.c |
16 | +++ b/configure | 23 | +++ b/linux-user/syscall.c |
17 | @@ -XXX,XX +XXX,XX @@ werror="" | 24 | @@ -XXX,XX +XXX,XX @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, |
18 | 25 | #if defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME) | |
19 | meson_option_build_array() { | 26 | case TARGET_NR_clock_adjtime: |
20 | printf '[' | 27 | { |
21 | - (if test "$targetos" == windows; then | 28 | - struct timex htx, *phtx = &htx; |
22 | + (if test "$targetos" = windows; then | 29 | + struct timex htx; |
23 | IFS=\; | 30 | |
24 | else | 31 | - if (target_to_host_timex(phtx, arg2) != 0) { |
25 | IFS=: | 32 | + if (target_to_host_timex(&htx, arg2) != 0) { |
33 | return -TARGET_EFAULT; | ||
34 | } | ||
35 | - ret = get_errno(clock_adjtime(arg1, phtx)); | ||
36 | - if (!is_error(ret) && phtx) { | ||
37 | - if (host_to_target_timex(arg2, phtx) != 0) { | ||
38 | - return -TARGET_EFAULT; | ||
39 | - } | ||
40 | + ret = get_errno(clock_adjtime(arg1, &htx)); | ||
41 | + if (!is_error(ret) && host_to_target_timex(arg2, &htx)) { | ||
42 | + return -TARGET_EFAULT; | ||
43 | } | ||
44 | } | ||
45 | return ret; | ||
26 | -- | 46 | -- |
27 | 2.25.1 | 47 | 2.34.1 |
48 | |||
49 | diff view generated by jsdifflib |
1 | In commit 823eb013452e93d we moved the setting of ARCH from configure | 1 | Add comments to the in_* fields in the S1Translate struct |
---|---|---|---|
2 | to meson.build, but we accidentally left behind one attempt to use | 2 | that explain what they're doing. |
3 | $ARCH in configure, which was trying to add -msmall-data to the | ||
4 | compiler flags on Alpha hosts. Since ARCH is now never set, the test | ||
5 | always fails and we never add the flag. | ||
6 | |||
7 | There isn't actually any need to use this compiler flag on Alpha: | ||
8 | the original intent was that it would allow us to simplify our TCG | ||
9 | codegen on that platform, but we never actually made the TCG changes | ||
10 | that would rely on -msmall-data. | ||
11 | |||
12 | Drop the effectively-dead code from configure, as we don't need it. | ||
13 | |||
14 | This was spotted by shellcheck: | ||
15 | |||
16 | In ./configure line 2254: | ||
17 | case "$ARCH" in | ||
18 | ^---^ SC2153: Possible misspelling: ARCH may not be assigned, but arch is. | ||
19 | 3 | ||
20 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 4 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
21 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 5 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
22 | Message-id: 20220720152631.450903-5-peter.maydell@linaro.org | 6 | Message-id: 20230710152130.3928330-2-peter.maydell@linaro.org |
23 | --- | 7 | --- |
24 | configure | 7 ------- | 8 | target/arm/ptw.c | 40 ++++++++++++++++++++++++++++++++++++++++ |
25 | 1 file changed, 7 deletions(-) | 9 | 1 file changed, 40 insertions(+) |
26 | 10 | ||
27 | diff --git a/configure b/configure | 11 | diff --git a/target/arm/ptw.c b/target/arm/ptw.c |
28 | index XXXXXXX..XXXXXXX 100755 | 12 | index XXXXXXX..XXXXXXX 100644 |
29 | --- a/configure | 13 | --- a/target/arm/ptw.c |
30 | +++ b/configure | 14 | +++ b/target/arm/ptw.c |
31 | @@ -XXX,XX +XXX,XX @@ if test "$fortify_source" = "yes" ; then | 15 | @@ -XXX,XX +XXX,XX @@ |
32 | QEMU_CFLAGS="-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $QEMU_CFLAGS" | 16 | #endif |
33 | fi | 17 | |
34 | 18 | typedef struct S1Translate { | |
35 | -case "$ARCH" in | 19 | + /* |
36 | -alpha) | 20 | + * in_mmu_idx : specifies which TTBR, TCR, etc to use for the walk. |
37 | - # Ensure there's only a single GP | 21 | + * Together with in_space, specifies the architectural translation regime. |
38 | - QEMU_CFLAGS="-msmall-data $QEMU_CFLAGS" | 22 | + */ |
39 | -;; | 23 | ARMMMUIdx in_mmu_idx; |
40 | -esac | 24 | + /* |
41 | - | 25 | + * in_ptw_idx: specifies which mmuidx to use for the actual |
42 | if test "$have_asan" = "yes"; then | 26 | + * page table descriptor load operations. This will be one of the |
43 | QEMU_CFLAGS="-fsanitize=address $QEMU_CFLAGS" | 27 | + * ARMMMUIdx_Stage2* or one of the ARMMMUIdx_Phys_* indexes. |
44 | QEMU_LDFLAGS="-fsanitize=address $QEMU_LDFLAGS" | 28 | + * If a Secure ptw is "downgraded" to NonSecure by an NSTable bit, |
29 | + * this field is updated accordingly. | ||
30 | + */ | ||
31 | ARMMMUIdx in_ptw_idx; | ||
32 | + /* | ||
33 | + * in_space: the security space for this walk. This plus | ||
34 | + * the in_mmu_idx specify the architectural translation regime. | ||
35 | + * If a Secure ptw is "downgraded" to NonSecure by an NSTable bit, | ||
36 | + * this field is updated accordingly. | ||
37 | + * | ||
38 | + * Note that the security space for the in_ptw_idx may be different | ||
39 | + * from that for the in_mmu_idx. We do not need to explicitly track | ||
40 | + * the in_ptw_idx security space because: | ||
41 | + * - if the in_ptw_idx is an ARMMMUIdx_Phys_* then the mmuidx | ||
42 | + * itself specifies the security space | ||
43 | + * - if the in_ptw_idx is an ARMMMUIdx_Stage2* then the security | ||
44 | + * space used for ptw reads is the same as that of the security | ||
45 | + * space of the stage 1 translation for all cases except where | ||
46 | + * stage 1 is Secure; in that case the only possibilities for | ||
47 | + * the ptw read are Secure and NonSecure, and the in_ptw_idx | ||
48 | + * value being Stage2 vs Stage2_S distinguishes those. | ||
49 | + */ | ||
50 | ARMSecuritySpace in_space; | ||
51 | + /* | ||
52 | + * in_secure: whether the translation regime is a Secure one. | ||
53 | + * This is always equal to arm_space_is_secure(in_space). | ||
54 | + * If a Secure ptw is "downgraded" to NonSecure by an NSTable bit, | ||
55 | + * this field is updated accordingly. | ||
56 | + */ | ||
57 | bool in_secure; | ||
58 | + /* | ||
59 | + * in_debug: is this a QEMU debug access (gdbstub, etc)? Debug | ||
60 | + * accesses will not update the guest page table access flags | ||
61 | + * and will not change the state of the softmmu TLBs. | ||
62 | + */ | ||
63 | bool in_debug; | ||
64 | /* | ||
65 | * If this is stage 2 of a stage 1+2 page table walk, then this must | ||
45 | -- | 66 | -- |
46 | 2.25.1 | 67 | 2.34.1 | diff view generated by jsdifflib |
1 | In commit 7390e0e9ab8475, we added support for SME loads and stores. | 1 | In commit fe4a5472ccd6 we rearranged the logic in S1_ptw_translate() |
---|---|---|---|
2 | Unlike SVE loads and stores, these include handling of 128-bit | 2 | so that the debug-access "call get_phys_addr_*" codepath is used both |
3 | elements. The SME load/store functions call down into the existing | 3 | when S1 is doing ptw reads from stage 2 and when it is doing ptw |
4 | sve_cont_ldst_elements() function, which uses the element size MO_* | 4 | reads from physical memory. However, we didn't update the |
5 | value as an index into the pred_esz_masks[] array. Because this code | 5 | calculation of s2ptw->in_space and s2ptw->in_secure to account for |
6 | path now has to handle MO_128, we need to add an extra element to the | 6 | the "ptw reads from physical memory" case. This meant that debug |
7 | array. | 7 | accesses when in Secure state broke. |
8 | 8 | ||
9 | This bug was spotted by Coverity because it meant we were reading off | 9 | Create a new function S2_security_space() which returns the |
10 | the end of the array. | 10 | correct security space to use for the ptw load, and use it to |
11 | determine the correct .in_secure and .in_space fields for the | ||
12 | stage 2 lookup for the ptw load. | ||
11 | 13 | ||
12 | Resolves: Coverity CID 1490539, 1490541, 1490543, 1490544, 1490545, | 14 | Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org> |
13 | 1490546, 1490548, 1490549, 1490550, 1490551, 1490555, 1490557, | ||
14 | 1490558, 1490560, 1490561, 1490563 | ||
15 | Fixes: 7390e0e9ab8475 ("target/arm: Implement SME LD1, ST1") | ||
16 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 15 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
16 | Tested-by: Jean-Philippe Brucker <jean-philippe@linaro.org> | ||
17 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | 17 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> |
18 | Message-id: 20220718100144.3248052-1-peter.maydell@linaro.org | 18 | Message-id: 20230710152130.3928330-3-peter.maydell@linaro.org |
19 | Fixes: fe4a5472ccd6 ("target/arm: Use get_phys_addr_with_struct in S1_ptw_translate") | ||
20 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | ||
19 | --- | 21 | --- |
20 | target/arm/cpu.h | 2 +- | 22 | target/arm/ptw.c | 37 ++++++++++++++++++++++++++++++++----- |
21 | target/arm/translate-sve.c | 5 +++-- | 23 | 1 file changed, 32 insertions(+), 5 deletions(-) |
22 | 2 files changed, 4 insertions(+), 3 deletions(-) | ||
23 | 24 | ||
24 | diff --git a/target/arm/cpu.h b/target/arm/cpu.h | 25 | diff --git a/target/arm/ptw.c b/target/arm/ptw.c |
25 | index XXXXXXX..XXXXXXX 100644 | 26 | index XXXXXXX..XXXXXXX 100644 |
26 | --- a/target/arm/cpu.h | 27 | --- a/target/arm/ptw.c |
27 | +++ b/target/arm/cpu.h | 28 | +++ b/target/arm/ptw.c |
28 | @@ -XXX,XX +XXX,XX @@ static inline uint64_t *aa64_vfp_qreg(CPUARMState *env, unsigned regno) | 29 | @@ -XXX,XX +XXX,XX @@ static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs) |
30 | } | ||
29 | } | 31 | } |
30 | 32 | ||
31 | /* Shared between translate-sve.c and sve_helper.c. */ | 33 | +static ARMSecuritySpace S2_security_space(ARMSecuritySpace s1_space, |
32 | -extern const uint64_t pred_esz_masks[4]; | 34 | + ARMMMUIdx s2_mmu_idx) |
33 | +extern const uint64_t pred_esz_masks[5]; | 35 | +{ |
34 | 36 | + /* | |
35 | /* Helper for the macros below, validating the argument type. */ | 37 | + * Return the security space to use for stage 2 when doing |
36 | static inline MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x) | 38 | + * the S1 page table descriptor load. |
37 | diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c | 39 | + */ |
38 | index XXXXXXX..XXXXXXX 100644 | 40 | + if (regime_is_stage2(s2_mmu_idx)) { |
39 | --- a/target/arm/translate-sve.c | 41 | + /* |
40 | +++ b/target/arm/translate-sve.c | 42 | + * The security space for ptw reads is almost always the same |
41 | @@ -XXX,XX +XXX,XX @@ static void do_predtest(DisasContext *s, int dofs, int gofs, int words) | 43 | + * as that of the security space of the stage 1 translation. |
42 | } | 44 | + * The only exception is when stage 1 is Secure; in that case |
43 | 45 | + * the ptw read might be to the Secure or the NonSecure space | |
44 | /* For each element size, the bits within a predicate word that are active. */ | 46 | + * (but never Realm or Root), and the s2_mmu_idx tells us which. |
45 | -const uint64_t pred_esz_masks[4] = { | 47 | + * Root translations are always single-stage. |
46 | +const uint64_t pred_esz_masks[5] = { | 48 | + */ |
47 | 0xffffffffffffffffull, 0x5555555555555555ull, | 49 | + if (s1_space == ARMSS_Secure) { |
48 | - 0x1111111111111111ull, 0x0101010101010101ull | 50 | + return arm_secure_to_space(s2_mmu_idx == ARMMMUIdx_Stage2_S); |
49 | + 0x1111111111111111ull, 0x0101010101010101ull, | 51 | + } else { |
50 | + 0x0001000100010001ull, | 52 | + assert(s2_mmu_idx != ARMMMUIdx_Stage2_S); |
51 | }; | 53 | + assert(s1_space != ARMSS_Root); |
52 | 54 | + return s1_space; | |
53 | static bool trans_INVALID(DisasContext *s, arg_INVALID *a) | 55 | + } |
56 | + } else { | ||
57 | + /* ptw loads are from phys: the mmu idx itself says which space */ | ||
58 | + return arm_phys_to_space(s2_mmu_idx); | ||
59 | + } | ||
60 | +} | ||
61 | + | ||
62 | /* Translate a S1 pagetable walk through S2 if needed. */ | ||
63 | static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, | ||
64 | hwaddr addr, ARMMMUFaultInfo *fi) | ||
65 | { | ||
66 | - ARMSecuritySpace space = ptw->in_space; | ||
67 | bool is_secure = ptw->in_secure; | ||
68 | ARMMMUIdx mmu_idx = ptw->in_mmu_idx; | ||
69 | ARMMMUIdx s2_mmu_idx = ptw->in_ptw_idx; | ||
70 | @@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw, | ||
71 | * From gdbstub, do not use softmmu so that we don't modify the | ||
72 | * state of the cpu at all, including softmmu tlb contents. | ||
73 | */ | ||
74 | + ARMSecuritySpace s2_space = S2_security_space(ptw->in_space, s2_mmu_idx); | ||
75 | S1Translate s2ptw = { | ||
76 | .in_mmu_idx = s2_mmu_idx, | ||
77 | .in_ptw_idx = ptw_idx_for_stage_2(env, s2_mmu_idx), | ||
78 | - .in_secure = s2_mmu_idx == ARMMMUIdx_Stage2_S, | ||
79 | - .in_space = (s2_mmu_idx == ARMMMUIdx_Stage2_S ? ARMSS_Secure | ||
80 | - : space == ARMSS_Realm ? ARMSS_Realm | ||
81 | - : ARMSS_NonSecure), | ||
82 | + .in_secure = arm_space_is_secure(s2_space), | ||
83 | + .in_space = s2_space, | ||
84 | .in_debug = true, | ||
85 | }; | ||
86 | GetPhysAddrResult s2 = { }; | ||
54 | -- | 87 | -- |
55 | 2.25.1 | 88 | 2.34.1 | diff view generated by jsdifflib |
1 | The variable string-replacement syntax ${var/old/new} is a bashism | 1 | In get_phys_addr_twostage() the code that applies the effects of |
---|---|---|---|
2 | (though it is also supported by some other shells), and for instance | 2 | VSTCR.{SA,SW} and VTCR.{NSA,NSW} only updates result->f.attrs.secure. |
3 | does not work with the NetBSD /bin/sh, which complains: | 3 | Now we also have f.attrs.space for FEAT_RME, we need to keep the two |
4 | ../src/configure: 687: Syntax error: Bad substitution | 4 | in sync. |
5 | 5 | ||
6 | Replace it with a more portable sed-based approach, similar to | 6 | These bits only have an effect for Secure space translations, not |
7 | what we already do in quote_sh(). | 7 | for Root, so use the input in_space field to determine whether to |
8 | apply them rather than the input is_secure. This doesn't actually | ||
9 | make a difference because Root translations are never two-stage, | ||
10 | but it's a little clearer. | ||
8 | 11 | ||
9 | Note that shellcheck also diagnoses this: | 12 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
13 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
14 | Message-id: 20230710152130.3928330-4-peter.maydell@linaro.org | ||
15 | --- | ||
16 | target/arm/ptw.c | 13 ++++++++----- | ||
17 | 1 file changed, 8 insertions(+), 5 deletions(-) | ||
10 | 18 | ||
11 | In ./configure line 687: | 19 | diff --git a/target/arm/ptw.c b/target/arm/ptw.c |
12 | e=${e/'\'/'\\'} | 20 | index XXXXXXX..XXXXXXX 100644 |
13 | ^-----------^ SC2039: In POSIX sh, string replacement is undefined. | 21 | --- a/target/arm/ptw.c |
14 | ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'. | 22 | +++ b/target/arm/ptw.c |
15 | ^-- SC1003: Want to escape a single quote? echo 'This is how it'\''s done'. | 23 | @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, |
16 | 24 | hwaddr ipa; | |
17 | In ./configure line 688: | 25 | int s1_prot, s1_lgpgsz; |
18 | e=${e/\"/'\"'} | 26 | bool is_secure = ptw->in_secure; |
19 | ^----------^ SC2039: In POSIX sh, string replacement is undefined. | 27 | + ARMSecuritySpace in_space = ptw->in_space; |
20 | 28 | bool ret, ipa_secure; | |
21 | Fixes: 8154f5e64b0cf ("meson: Prefix each element of firmware path") | 29 | ARMCacheAttrs cacheattrs1; |
22 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 30 | ARMSecuritySpace ipa_space; |
23 | Tested-by: Thomas Huth <thuth@redhat.com> | 31 | @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_twostage(CPUARMState *env, S1Translate *ptw, |
24 | Message-id: 20220720152631.450903-4-peter.maydell@linaro.org | 32 | * Check if IPA translates to secure or non-secure PA space. |
25 | --- | 33 | * Note that VSTCR overrides VTCR and {N}SW overrides {N}SA. |
26 | configure | 7 ++++--- | 34 | */ |
27 | 1 file changed, 4 insertions(+), 3 deletions(-) | 35 | - result->f.attrs.secure = |
28 | 36 | - (is_secure | |
29 | diff --git a/configure b/configure | 37 | - && !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)) |
30 | index XXXXXXX..XXXXXXX 100755 | 38 | - && (ipa_secure |
31 | --- a/configure | 39 | - || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW)))); |
32 | +++ b/configure | 40 | + if (in_space == ARMSS_Secure) { |
33 | @@ -XXX,XX +XXX,XX @@ meson_option_build_array() { | 41 | + result->f.attrs.secure = |
34 | IFS=: | 42 | + !(env->cp15.vstcr_el2 & (VSTCR_SA | VSTCR_SW)) |
35 | fi | 43 | + && (ipa_secure |
36 | for e in $1; do | 44 | + || !(env->cp15.vtcr_el2 & (VTCR_NSA | VTCR_NSW))); |
37 | - e=${e/'\'/'\\'} | 45 | + result->f.attrs.space = arm_secure_to_space(result->f.attrs.secure); |
38 | - e=${e/\"/'\"'} | 46 | + } |
39 | - printf '"""%s""",' "$e" | 47 | |
40 | + printf '"""' | 48 | return false; |
41 | + # backslash escape any '\' and '"' characters | ||
42 | + printf "%s" "$e" | sed -e 's/\([\"]\)/\\\1/g' | ||
43 | + printf '""",' | ||
44 | done) | ||
45 | printf ']\n' | ||
46 | } | 49 | } |
47 | -- | 50 | -- |
48 | 2.25.1 | 51 | 2.34.1 | diff view generated by jsdifflib |
1 | In shell script syntax, $var[something] is not special for variable | 1 | In commit f0a08b0913befbd we changed the type of the PC from |
---|---|---|---|
2 | expansion: $var is expanded. However, as it can look as if it were | 2 | target_ulong to vaddr. In doing so we inadvertently dropped the |
3 | intended to be an array element access (the correct syntax for which | 3 | zero-padding on the PC in trace lines (the second item inside the [] |
4 | is ${var[something]}), shellcheck recommends using explicit braces | 4 | in these lines). They used to look like this on AArch64, for |
5 | around ${var} to clarify the intended expansion. | 5 | instance: |
6 | 6 | ||
7 | This fixes the warning: | 7 | Trace 0: 0x7f2260000100 [00000000/0000000040000000/00000061/ff200000] |
8 | 8 | ||
9 | In ./configure line 2346: | 9 | and now they look like this: |
10 | if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then | 10 | Trace 0: 0x7f4f50000100 [00000000/40000000/00000061/ff200000] |
11 | ^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet). | ||
12 | 11 | ||
12 | and if the PC happens to be somewhere low like 0x5000 | ||
13 | then the field is shown as /5000/. | ||
14 | |||
15 | This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier, | ||
16 | depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64 | ||
17 | with no width specifier. | ||
18 | |||
19 | Restore the zero-padding by adding an 016 width specifier to | ||
20 | this tracing and a couple of others that were similarly recently | ||
21 | changed to use VADDR_PRIx without a width specifier. | ||
22 | |||
23 | We can't unfortunately restore the "32-bit guests are padded to | ||
24 | 8 hex digits and 64-bit guests to 16 hex digits" behaviour so | ||
25 | easily. | ||
26 | |||
27 | Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr") | ||
13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 28 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
14 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 29 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
15 | Message-id: 20220720152631.450903-3-peter.maydell@linaro.org | 30 | Reviewed-by: Anton Johansson <anjo@rev.ng> |
31 | Message-id: 20230711165434.4123674-1-peter.maydell@linaro.org | ||
16 | --- | 32 | --- |
17 | configure | 2 +- | 33 | accel/tcg/cpu-exec.c | 4 ++-- |
18 | 1 file changed, 1 insertion(+), 1 deletion(-) | 34 | accel/tcg/translate-all.c | 2 +- |
35 | 2 files changed, 3 insertions(+), 3 deletions(-) | ||
19 | 36 | ||
20 | diff --git a/configure b/configure | 37 | diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c |
21 | index XXXXXXX..XXXXXXX 100755 | 38 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/configure | 39 | --- a/accel/tcg/cpu-exec.c |
23 | +++ b/configure | 40 | +++ b/accel/tcg/cpu-exec.c |
24 | @@ -XXX,XX +XXX,XX @@ if test -n "$target_cc" && | 41 | @@ -XXX,XX +XXX,XX @@ static void log_cpu_exec(vaddr pc, CPUState *cpu, |
25 | # emulation. Linux and OpenBSD/amd64 use 'elf_i386'; FreeBSD uses the _fbsd | 42 | if (qemu_log_in_addr_range(pc)) { |
26 | # variant; OpenBSD/i386 uses the _obsd variant; and Windows uses i386pe. | 43 | qemu_log_mask(CPU_LOG_EXEC, |
27 | for emu in elf_i386 elf_i386_fbsd elf_i386_obsd i386pe; do | 44 | "Trace %d: %p [%08" PRIx64 |
28 | - if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*$emu[[:space:]]*$"; then | 45 | - "/%" VADDR_PRIx "/%08x/%08x] %s\n", |
29 | + if "$target_ld" -verbose 2>&1 | grep -q "^[[:space:]]*${emu}[[:space:]]*$"; then | 46 | + "/%016" VADDR_PRIx "/%08x/%08x] %s\n", |
30 | ld_i386_emulation="$emu" | 47 | cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc, |
31 | break | 48 | tb->flags, tb->cflags, lookup_symbol(pc)); |
32 | fi | 49 | |
50 | @@ -XXX,XX +XXX,XX @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit) | ||
51 | if (qemu_loglevel_mask(CPU_LOG_EXEC)) { | ||
52 | vaddr pc = log_pc(cpu, last_tb); | ||
53 | if (qemu_log_in_addr_range(pc)) { | ||
54 | - qemu_log("Stopped execution of TB chain before %p [%" | ||
55 | + qemu_log("Stopped execution of TB chain before %p [%016" | ||
56 | VADDR_PRIx "] %s\n", | ||
57 | last_tb->tc.ptr, pc, lookup_symbol(pc)); | ||
58 | } | ||
59 | diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c | ||
60 | index XXXXXXX..XXXXXXX 100644 | ||
61 | --- a/accel/tcg/translate-all.c | ||
62 | +++ b/accel/tcg/translate-all.c | ||
63 | @@ -XXX,XX +XXX,XX @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) | ||
64 | if (qemu_loglevel_mask(CPU_LOG_EXEC)) { | ||
65 | vaddr pc = log_pc(cpu, tb); | ||
66 | if (qemu_log_in_addr_range(pc)) { | ||
67 | - qemu_log("cpu_io_recompile: rewound execution of TB to %" | ||
68 | + qemu_log("cpu_io_recompile: rewound execution of TB to %016" | ||
69 | VADDR_PRIx "\n", pc); | ||
70 | } | ||
71 | } | ||
33 | -- | 72 | -- |
34 | 2.25.1 | 73 | 2.34.1 |
74 | |||
75 | diff view generated by jsdifflib |
1 | In commit 7d7dbf9dc15be6e1 we added a line to the configure script | 1 | From: Tong Ho <tong.ho@amd.com> |
---|---|---|---|
2 | which is not valid POSIX shell syntax, because it is missing a space | ||
3 | after a '!' character. shellcheck diagnoses this: | ||
4 | 2 | ||
5 | if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then | 3 | Add a check in the bit-set operation to write the backstore |
6 | ^-- SC1035: You are missing a required space after the !. | 4 | only if the affected bit is 0 before. |
7 | 5 | ||
8 | and the OpenBSD shell will not correctly handle this without the space. | 6 | With this in place, there will be no need for callers to |
7 | do the checking in order to avoid unnecessary writes. | ||
9 | 8 | ||
10 | Fixes: 7d7dbf9dc15be6e1 ("configure: replace --enable/disable-git-update with --with-git-submodules") | 9 | Signed-off-by: Tong Ho <tong.ho@amd.com> |
10 | Reviewed-by: Alistair Francis <alistair.francis@wdc.com> | ||
11 | Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com> | ||
12 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> | ||
11 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 13 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> |
12 | Reviewed-by: Thomas Huth <thuth@redhat.com> | ||
13 | Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> | ||
14 | Message-id: 20220720152631.450903-2-peter.maydell@linaro.org | ||
15 | --- | 14 | --- |
16 | configure | 2 +- | 15 | hw/nvram/xlnx-efuse.c | 11 +++++++++-- |
17 | 1 file changed, 1 insertion(+), 1 deletion(-) | 16 | 1 file changed, 9 insertions(+), 2 deletions(-) |
18 | 17 | ||
19 | diff --git a/configure b/configure | 18 | diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c |
20 | index XXXXXXX..XXXXXXX 100755 | 19 | index XXXXXXX..XXXXXXX 100644 |
21 | --- a/configure | 20 | --- a/hw/nvram/xlnx-efuse.c |
22 | +++ b/configure | 21 | +++ b/hw/nvram/xlnx-efuse.c |
23 | @@ -XXX,XX +XXX,XX @@ else | 22 | @@ -XXX,XX +XXX,XX @@ static bool efuse_ro_bits_find(XlnxEFuse *s, uint32_t k) |
24 | cxx= | 23 | |
25 | fi | 24 | bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit) |
26 | 25 | { | |
27 | -if !(GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then | 26 | + uint32_t set, *row; |
28 | +if ! (GIT="$git" "$source_path/scripts/git-submodule.sh" "$git_submodules_action" "$git_submodules"); then | 27 | + |
29 | exit 1 | 28 | if (efuse_ro_bits_find(s, bit)) { |
30 | fi | 29 | g_autofree char *path = object_get_canonical_path(OBJECT(s)); |
30 | |||
31 | @@ -XXX,XX +XXX,XX @@ bool xlnx_efuse_set_bit(XlnxEFuse *s, unsigned int bit) | ||
32 | return false; | ||
33 | } | ||
34 | |||
35 | - s->fuse32[bit / 32] |= 1 << (bit % 32); | ||
36 | - efuse_bdrv_sync(s, bit); | ||
37 | + /* Avoid back-end write unless there is a real update */ | ||
38 | + row = &s->fuse32[bit / 32]; | ||
39 | + set = 1 << (bit % 32); | ||
40 | + if (!(set & *row)) { | ||
41 | + *row |= set; | ||
42 | + efuse_bdrv_sync(s, bit); | ||
43 | + } | ||
44 | return true; | ||
45 | } | ||
31 | 46 | ||
32 | -- | 47 | -- |
33 | 2.25.1 | 48 | 2.34.1 |
49 | |||
50 | diff view generated by jsdifflib |