A last small test of bug fixes before rc1.

The following changes since commit ed8ad9728a9c0eec34db9dff61dfa2f1dd625637:

Merge tag 'pull-tpm-2023-07-14-1' of https://github.com/stefanberger/qemu-tpm into staging (2023-07-15 14:54:04 +0100)

are available in the Git repository at:

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20230717

for you to fetch changes up to c2c1c4a35c7c2b1a4140b0942b9797c857e476a4:

hw/nvram: Avoid unnecessary Xilinx eFuse backstore write (2023-07-17 11:05:52 +0100)

* hw/arm/sbsa-ref: set 'slots' property of xhci

* linux-user: Remove pointless NULL check in clock_adjtime handling

* ptw: Fix S1_ptw_translate() debug path

* ptw: Account for FEAT_RME when applying {N}SW, SA bits

* accel/tcg: Zero-pad PC in TCG CPU exec trace lines

* hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Peter Maydell (5):

linux-user: Remove pointless NULL check in clock_adjtime handling

target/arm/ptw.c: Add comments to S1Translate struct fields

target/arm: Fix S1_ptw_translate() debug path

target/arm/ptw.c: Account for FEAT_RME when applying {N}SW, SA bits

accel/tcg: Zero-pad PC in TCG CPU exec trace lines

Tong Ho (1):

hw/nvram: Avoid unnecessary Xilinx eFuse backstore write

Yuquan Wang (1):

hw/arm/sbsa-ref: set 'slots' property of xhci

accel/tcg/cpu-exec.c | 4 +--

accel/tcg/translate-all.c | 2 +-

hw/arm/sbsa-ref.c | 1 +

hw/nvram/xlnx-efuse.c | 11 ++++--

linux-user/syscall.c | 12 +++----

target/arm/ptw.c | 90 +++++++++++++++++++++++++++++++++++++++++------

6 files changed, 98 insertions(+), 22 deletions(-)

From: Yuquan Wang <wangyuquan1236@phytium.com.cn>

This extends the slots of xhci to 64, since the default xhci_sysbus

just supports one slot.

Signed-off-by: Wang Yuquan <wangyuquan1236@phytium.com.cn>

Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Tested-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>

Message-id: 20230710063750.473510-2-wangyuquan1236@phytium.com.cn

hw/arm/sbsa-ref.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c

--- a/hw/arm/sbsa-ref.c

+++ b/hw/arm/sbsa-ref.c

@@ -XXX,XX +XXX,XX @@ static void create_xhci(const SBSAMachineState *sms)

hwaddr base = sbsa_ref_memmap[SBSA_XHCI].base;

int irq = sbsa_ref_irqmap[SBSA_XHCI];

DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS);

+ qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS);

sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);

sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);

2.34.1

In the code for TARGET_NR_clock_adjtime, we set the pointer phtx to

the address of the local variable htx. This means it can never be

NULL, but later in the code we check it for NULL anyway. Coverity

complains about this (CID 1507683) because the NULL check comes after

a call to clock_adjtime() that assumes it is non-NULL.

Since phtx is always &htx, and is used only in three places, it's not

really necessary. Remove it, bringing the code structure in to line

with that for TARGET_NR_clock_adjtime64, which already uses a simple

'&htx' when it wants a pointer to 'htx'.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20230623144410.1837261-1-peter.maydell@linaro.org

---

linux-user/syscall.c | 12 +++++-------

1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c

--- a/linux-user/syscall.c

+++ b/linux-user/syscall.c

@@ -XXX,XX +XXX,XX @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1,

#if defined(TARGET_NR_clock_adjtime) && defined(CONFIG_CLOCK_ADJTIME)

case TARGET_NR_clock_adjtime:

{

- struct timex htx, *phtx = &htx;

+ struct timex htx;

- if (target_to_host_timex(phtx, arg2) != 0) {

+ if (target_to_host_timex(&htx, arg2) != 0) {

return -TARGET_EFAULT;

}

- ret = get_errno(clock_adjtime(arg1, phtx));

- if (!is_error(ret) && phtx) {

- if (host_to_target_timex(arg2, phtx) != 0) {

- return -TARGET_EFAULT;

- }

+ ret = get_errno(clock_adjtime(arg1, &htx));

+ if (!is_error(ret) && host_to_target_timex(arg2, &htx)) {

+ return -TARGET_EFAULT;

}

}

return ret;

2.34.1

Add comments to the in_* fields in the S1Translate struct

that explain what they're doing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20230710152130.3928330-2-peter.maydell@linaro.org

---

target/arm/ptw.c | 40 ++++++++++++++++++++++++++++++++++++++++

1 file changed, 40 insertions(+)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c

--- a/target/arm/ptw.c

+++ b/target/arm/ptw.c

@@ -XXX,XX +XXX,XX @@

2.34.1

In commit fe4a5472ccd6 we rearranged the logic in S1_ptw_translate()

so that the debug-access "call get_phys_addr_*" codepath is used both

when S1 is doing ptw reads from stage 2 and when it is doing ptw

reads from physical memory. However, we didn't update the

calculation of s2ptw->in_space and s2ptw->in_secure to account for

the "ptw reads from physical memory" case. This meant that debug

accesses when in Secure state broke.

Create a new function S2_security_space() which returns the

correct security space to use for the ptw load, and use it to

determine the correct .in_secure and .in_space fields for the

stage 2 lookup for the ptw load.

Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Tested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20230710152130.3928330-3-peter.maydell@linaro.org

Fixes: fe4a5472ccd6 ("target/arm: Use get_phys_addr_with_struct in S1_ptw_translate")

target/arm/ptw.c | 37 ++++++++++++++++++++++++++++++++-----

1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c

--- a/target/arm/ptw.c

+++ b/target/arm/ptw.c

@@ -XXX,XX +XXX,XX @@ static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs)

+static ARMSecuritySpace S2_security_space(ARMSecuritySpace s1_space,

+ ARMMMUIdx s2_mmu_idx)

+ /*

+ * Return the security space to use for stage 2 when doing

+ * the S1 page table descriptor load.

+ */

+ if (regime_is_stage2(s2_mmu_idx)) {

+ /*

+ * The security space for ptw reads is almost always the same

+ * as that of the security space of the stage 1 translation.

+ * The only exception is when stage 1 is Secure; in that case

+ * the ptw read might be to the Secure or the NonSecure space

+ * (but never Realm or Root), and the s2_mmu_idx tells us which.

+ * Root translations are always single-stage.

+ */

+ if (s1_space == ARMSS_Secure) {

+ return arm_secure_to_space(s2_mmu_idx == ARMMMUIdx_Stage2_S);

+ } else {

+ assert(s2_mmu_idx != ARMMMUIdx_Stage2_S);

+ assert(s1_space != ARMSS_Root);

+ return s1_space;

+ }

+ } else {

+ /* ptw loads are from phys: the mmu idx itself says which space */

+ return arm_phys_to_space(s2_mmu_idx);

/* Translate a S1 pagetable walk through S2 if needed. */

static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,

hwaddr addr, ARMMMUFaultInfo *fi)

- ARMSecuritySpace space = ptw->in_space;

bool is_secure = ptw->in_secure;

ARMMMUIdx mmu_idx = ptw->in_mmu_idx;

ARMMMUIdx s2_mmu_idx = ptw->in_ptw_idx;

@@ -XXX,XX +XXX,XX @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,

* From gdbstub, do not use softmmu so that we don't modify the

* state of the cpu at all, including softmmu tlb contents.

*/

+ ARMSecuritySpace s2_space = S2_security_space(ptw->in_space, s2_mmu_idx);

S1Translate s2ptw = {

.in_mmu_idx = s2_mmu_idx,

.in_ptw_idx = ptw_idx_for_stage_2(env, s2_mmu_idx),

- .in_secure = s2_mmu_idx == ARMMMUIdx_Stage2_S,

- .in_space = (s2_mmu_idx == ARMMMUIdx_Stage2_S ? ARMSS_Secure

- : space == ARMSS_Realm ? ARMSS_Realm

- : ARMSS_NonSecure),

+ .in_secure = arm_space_is_secure(s2_space),

+ .in_space = s2_space,

.in_debug = true,

};

GetPhysAddrResult s2 = { };

2.34.1