Peter Maydell (15):

target/arm/ptw: Don't set fi->s1ptw for UnsuppAtomicUpdate fault

target/arm/ptw: Don't report GPC faults on stage 1 ptw as stage2 faults

target/arm/ptw: Set s1ns bit in fault info more consistently

target/arm/ptw: Pass ptw into get_phys_addr_pmsa*() and get_phys_addr_disabled()

target/arm/ptw: Pass ARMSecurityState to regime_translation_disabled()

target/arm/ptw: Pass an ARMSecuritySpace to arm_hcr_el2_eff_secstate()

target/arm: Pass an ARMSecuritySpace to arm_is_el2_enabled_secstate()

target/arm/ptw: Only fold in NSTable bit effects in Secure state

target/arm/ptw: Remove last uses of ptw->in_secure

target/arm/ptw: Remove S1Translate::in_secure

target/arm/ptw: Drop S1Translate::out_secure

target/arm/ptw: Set attributes correctly for MMU disabled data accesses

target/arm/ptw: Check for block descriptors at invalid levels

target/arm/ptw: Report stage 2 fault level for stage 2 faults on stage 1 ptw

target/arm: Adjust PAR_EL1.SH for Device and Normal-NC memory types

Richard Henderson (2):

target/arm: Fix SME ST1Q

target/arm: Fix 64-bit SSRA

include/hw/gpio/nrf51_gpio.h | 1 +

include/sysemu/kvm.h | 2 +

target/arm/cpu.h | 19 ++--

target/arm/internals.h | 25 ++---

target/mips/kvm_mips.h | 9 --

tests/qtest/libqtest.h | 11 +++

accel/kvm/kvm-all.c | 19 ++--

hw/arm/virt.c | 2 +-

hw/gpio/nrf51_gpio.c | 14 ++-

hw/mips/loongson3_virt.c | 2 -

hw/ppc/spapr.c | 2 +-

softmmu/qtest.c | 52 +++++++---

target/arm/cpu.c | 6 ++

target/arm/helper.c | 207 ++++++++++++++++++++++++++++----------

target/arm/kvm.c | 7 ++

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

target/arm/tcg/sme_helper.c | 2 +-

target/arm/tcg/translate.c | 2 +-

target/i386/kvm/kvm.c | 5 +

target/mips/kvm.c | 3 +-

target/ppc/kvm.c | 5 +

target/riscv/kvm.c | 5 +

target/s390x/kvm/kvm.c | 5 +

tests/qtest/libqtest.c | 6 ++

tests/qtest/microbit-test.c | 44 +++++++++

target/arm/trace-events | 7 +-

26 files changed, 494 insertions(+), 199 deletions(-)

From: Chris Laplante <chris@laplante.io>

Implement nRF51 DETECT signal in the GPIO peripheral.

The reference manual makes mention of a per-pin DETECT signal, but these

are not exposed to the user. See https://devzone.nordicsemi.com/f/nordic-q-a/39858/gpio-per-pin-detect-signal-available

for more information. Currently, I don't see a reason to model these.

Signed-off-by: Chris Laplante <chris@laplante.io>

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

Message-id: 20230728160324.1159090-2-chris@laplante.io

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

---

include/hw/gpio/nrf51_gpio.h | 1 +

hw/gpio/nrf51_gpio.c | 14 +++++++++++++-

2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/hw/gpio/nrf51_gpio.h b/include/hw/gpio/nrf51_gpio.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/gpio/nrf51_gpio.h

+++ b/include/hw/gpio/nrf51_gpio.h

@@ -XXX,XX +XXX,XX @@ struct NRF51GPIOState {

uint32_t old_out_connected;

qemu_irq output[NRF51_GPIO_PINS];

+ qemu_irq detect;

};

diff --git a/hw/gpio/nrf51_gpio.c b/hw/gpio/nrf51_gpio.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/gpio/nrf51_gpio.c

+++ b/hw/gpio/nrf51_gpio.c

@@ -XXX,XX +XXX,XX @@ static void update_state(NRF51GPIOState *s)

int pull;

size_t i;

bool connected_out, dir, connected_in, out, in, input;

+ bool assert_detect = false;

for (i = 0; i < NRF51_GPIO_PINS; i++) {

pull = pull_value(s->cnf[i]);

@@ -XXX,XX +XXX,XX @@ static void update_state(NRF51GPIOState *s)

qemu_log_mask(LOG_GUEST_ERROR,

"GPIO pin %zu short circuited\n", i);

}

- if (!connected_in) {

+ if (connected_in) {

+ uint32_t detect_config = extract32(s->cnf[i], 16, 2);

+ if ((detect_config == 2) && (in == 1)) {

+ assert_detect = true;

+ }

+ if ((detect_config == 3) && (in == 0)) {

+ assert_detect = true;

+ }

+ } else {

/*

* Floating input: the output stimulates IN if connected,

* otherwise pull-up/pull-down resistors put a value on both

@@ -XXX,XX +XXX,XX @@ static void update_state(NRF51GPIOState *s)

}

update_output_irq(s, i, connected_out, out);

}

+

+ qemu_set_irq(s->detect, assert_detect);

}

/*

@@ -XXX,XX +XXX,XX @@ static void nrf51_gpio_init(Object *obj)

qdev_init_gpio_in(DEVICE(s), nrf51_gpio_set, NRF51_GPIO_PINS);

qdev_init_gpio_out(DEVICE(s), s->output, NRF51_GPIO_PINS);

+ qdev_init_gpio_out_named(DEVICE(s), &s->detect, "detect", 1);

}

static void nrf51_gpio_class_init(ObjectClass *klass, void *data)

--

2.34.1

From: Chris Laplante <chris@laplante.io>

Signed-off-by: Chris Laplante <chris@laplante.io>

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

Message-id: 20230728160324.1159090-3-chris@laplante.io

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

---

softmmu/qtest.c | 16 ++++++++++------

1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c

index XXXXXXX..XXXXXXX 100644

--- a/softmmu/qtest.c

+++ b/softmmu/qtest.c

@@ -XXX,XX +XXX,XX @@ void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words))

process_command_cb = pc_cb;

}

+static void qtest_install_gpio_out_intercept(DeviceState *dev, const char *name, int n)

+{

+ qemu_irq *disconnected = g_new0(qemu_irq, 1);

+ qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,

+ disconnected, n);

+

+ *disconnected = qdev_intercept_gpio_out(dev, icpt, name, n);

+}

+

static void qtest_process_command(CharBackend *chr, gchar **words)

{

const gchar *command;

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

if (words[0][14] == 'o') {

int i;

for (i = 0; i < ngl->num_out; ++i) {

- qemu_irq *disconnected = g_new0(qemu_irq, 1);

- qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,

- disconnected, i);

-

- *disconnected = qdev_intercept_gpio_out(dev, icpt,

- ngl->name, i);

+ qtest_install_gpio_out_intercept(dev, ngl->name, i);

}

} else {

qemu_irq_intercept_in(ngl->in, qtest_irq_handler,

--

2.34.1

From: Chris Laplante <chris@laplante.io>

Adds qtest_irq_intercept_out_named method, which utilizes a new optional

name parameter to the irq_intercept_out qtest command.

Signed-off-by: Chris Laplante <chris@laplante.io>

Message-id: 20230728160324.1159090-4-chris@laplante.io

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

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

---

tests/qtest/libqtest.h | 11 +++++++++++

softmmu/qtest.c | 18 ++++++++++--------

tests/qtest/libqtest.c | 6 ++++++

3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h

index XXXXXXX..XXXXXXX 100644

--- a/tests/qtest/libqtest.h

+++ b/tests/qtest/libqtest.h

@@ -XXX,XX +XXX,XX @@ void qtest_irq_intercept_in(QTestState *s, const char *string);

*/

void qtest_irq_intercept_out(QTestState *s, const char *string);

+/**

+ * qtest_irq_intercept_out_named:

+ * @s: #QTestState instance to operate on.

+ * @qom_path: QOM path of a device.

+ * @name: Name of the GPIO out pin

+ *

+ * Associate a qtest irq with the named GPIO-out pin of the device

+ * whose path is specified by @string and whose name is @name.

+ */

+void qtest_irq_intercept_out_named(QTestState *s, const char *qom_path, const char *name);

+

/**

* qtest_set_irq_in:

* @s: QTestState instance to operate on.

diff --git a/softmmu/qtest.c b/softmmu/qtest.c

index XXXXXXX..XXXXXXX 100644

--- a/softmmu/qtest.c

+++ b/softmmu/qtest.c

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

|| strcmp(words[0], "irq_intercept_in") == 0) {

DeviceState *dev;

NamedGPIOList *ngl;

+ bool is_outbound;

g_assert(words[1]);

+ is_outbound = words[0][14] == 'o';

dev = DEVICE(object_resolve_path(words[1], NULL));

if (!dev) {

qtest_send_prefix(chr);

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

}

QLIST_FOREACH(ngl, &dev->gpios, node) {

- /* We don't support intercept of named GPIOs yet */

- if (ngl->name) {

- continue;

- }

- if (words[0][14] == 'o') {

- int i;

- for (i = 0; i < ngl->num_out; ++i) {

- qtest_install_gpio_out_intercept(dev, ngl->name, i);

+ /* We don't support inbound interception of named GPIOs yet */

+ if (is_outbound) {

+ /* NULL is valid and matchable, for "unnamed GPIO" */

+ if (g_strcmp0(ngl->name, words[2]) == 0) {

+ int i;

+ for (i = 0; i < ngl->num_out; ++i) {

+ qtest_install_gpio_out_intercept(dev, ngl->name, i);

+ }

}

} else {

qemu_irq_intercept_in(ngl->in, qtest_irq_handler,

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c

index XXXXXXX..XXXXXXX 100644

--- a/tests/qtest/libqtest.c

+++ b/tests/qtest/libqtest.c

@@ -XXX,XX +XXX,XX @@ void qtest_irq_intercept_out(QTestState *s, const char *qom_path)

qtest_rsp(s);

}

+void qtest_irq_intercept_out_named(QTestState *s, const char *qom_path, const char *name)

+{

+ qtest_sendf(s, "irq_intercept_out %s %s\n", qom_path, name);

+ qtest_rsp(s);

+}

+

void qtest_irq_intercept_in(QTestState *s, const char *qom_path)

{

qtest_sendf(s, "irq_intercept_in %s\n", qom_path);

--

2.34.1

From: Chris Laplante <chris@laplante.io>

Named interception of in-GPIOs is not supported yet.

Signed-off-by: Chris Laplante <chris@laplante.io>

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

Message-id: 20230728160324.1159090-5-chris@laplante.io

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

---

softmmu/qtest.c | 8 ++++++++

1 file changed, 8 insertions(+)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c

index XXXXXXX..XXXXXXX 100644

--- a/softmmu/qtest.c

+++ b/softmmu/qtest.c

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

|| strcmp(words[0], "irq_intercept_in") == 0) {

DeviceState *dev;

NamedGPIOList *ngl;

+ bool is_named;

bool is_outbound;

g_assert(words[1]);

+ is_named = words[2] != NULL;

is_outbound = words[0][14] == 'o';

dev = DEVICE(object_resolve_path(words[1], NULL));

if (!dev) {

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

return;

}

+ if (is_named && !is_outbound) {

+ qtest_send_prefix(chr);

+ qtest_send(chr, "FAIL Interception of named in-GPIOs not yet supported\n");

+ return;

+ }

+

if (irq_intercept_dev) {

qtest_send_prefix(chr);

if (irq_intercept_dev != dev) {

--

2.34.1

From: Chris Laplante <chris@laplante.io>

This is much better than just silently failing with OK.

Signed-off-by: Chris Laplante <chris@laplante.io>

Message-id: 20230728160324.1159090-6-chris@laplante.io

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

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

---

softmmu/qtest.c | 12 ++++++++++--

1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c

index XXXXXXX..XXXXXXX 100644

--- a/softmmu/qtest.c

+++ b/softmmu/qtest.c

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

NamedGPIOList *ngl;

bool is_named;

bool is_outbound;

+ bool interception_succeeded = false;

g_assert(words[1]);

is_named = words[2] != NULL;

@@ -XXX,XX +XXX,XX @@ static void qtest_process_command(CharBackend *chr, gchar **words)

for (i = 0; i < ngl->num_out; ++i) {

qtest_install_gpio_out_intercept(dev, ngl->name, i);

}

+ interception_succeeded = true;

}

} else {

qemu_irq_intercept_in(ngl->in, qtest_irq_handler,

ngl->num_in);

+ interception_succeeded = true;

}

}

- irq_intercept_dev = dev;

+

qtest_send_prefix(chr);

- qtest_send(chr, "OK\n");

+ if (interception_succeeded) {

+ irq_intercept_dev = dev;

+ qtest_send(chr, "OK\n");

+ } else {

+ qtest_send(chr, "FAIL No intercepts installed\n");

+ }

} else if (strcmp(words[0], "set_irq_in") == 0) {

DeviceState *dev;

qemu_irq irq;

--

2.34.1

From: Chris Laplante <chris@laplante.io>

Exercise the DETECT mechanism of the GPIO peripheral.

Signed-off-by: Chris Laplante <chris@laplante.io>

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

Message-id: 20230728160324.1159090-7-chris@laplante.io

[PMM: fixed coding style nits]

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

---

tests/qtest/microbit-test.c | 44 +++++++++++++++++++++++++++++++++++++

1 file changed, 44 insertions(+)

diff --git a/tests/qtest/microbit-test.c b/tests/qtest/microbit-test.c

index XXXXXXX..XXXXXXX 100644

--- a/tests/qtest/microbit-test.c

+++ b/tests/qtest/microbit-test.c

@@ -XXX,XX +XXX,XX @@ static void test_nrf51_gpio(void)

qtest_quit(qts);

}

+static void test_nrf51_gpio_detect(void)

+{

+ QTestState *qts = qtest_init("-M microbit");

+ int i;

+

+ /* Connect input buffer on pins 1-7, configure SENSE for high level */

+ for (i = 1; i <= 7; i++) {

+ qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START + i * 4,

+ deposit32(0, 16, 2, 2));

+ }

+

+ qtest_irq_intercept_out_named(qts, "/machine/nrf51/gpio", "detect");

+

+ for (i = 1; i <= 7; i++) {

+ /* Set pin high */

+ qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", i, 1);

+ uint32_t actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN);

+ g_assert_cmpuint(actual, ==, 1 << i);

+

+ /* Check that DETECT is high */

+ g_assert_true(qtest_get_irq(qts, 0));

+

+ /* Set pin low, check that DETECT goes low. */

+ qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", i, 0);

+ actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN);

+ g_assert_cmpuint(actual, ==, 0x0);

+ g_assert_false(qtest_get_irq(qts, 0));

+ }

+

+ /* Set pin 0 high, check that DETECT doesn't fire */

+ qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 1);

+ g_assert_false(qtest_get_irq(qts, 0));

+ qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 0);

+

+ /* Set pins 1, 2, and 3 high, then set 3 low. Check DETECT is still high */

+ for (i = 1; i <= 3; i++) {

+ qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", i, 1);

+ }

+ g_assert_true(qtest_get_irq(qts, 0));

+ qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 3, 0);

+ g_assert_true(qtest_get_irq(qts, 0));

+}

+

static void timer_task(QTestState *qts, hwaddr task)

{

qtest_writel(qts, NRF51_TIMER_BASE + task, NRF51_TRIGGER_TASK);

@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)

qtest_add_func("/microbit/nrf51/uart", test_nrf51_uart);

qtest_add_func("/microbit/nrf51/gpio", test_nrf51_gpio);

+ qtest_add_func("/microbit/nrf51/gpio_detect", test_nrf51_gpio_detect);

qtest_add_func("/microbit/nrf51/nvmc", test_nrf51_nvmc);

qtest_add_func("/microbit/nrf51/timer", test_nrf51_timer);

qtest_add_func("/microbit/microbit/i2c", test_microbit_i2c);

--

2.34.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Before this change, the default KVM type, which is used for non-virt

machine models, was 0.

The kernel documentation says:

> On arm64, the physical address size for a VM (IPA Size limit) is

> limited to 40bits by default. The limit can be configured if the host

> supports the extension KVM_CAP_ARM_VM_IPA_SIZE. When supported, use

> KVM_VM_TYPE_ARM_IPA_SIZE(IPA_Bits) to set the size in the machine type

> identifier, where IPA_Bits is the maximum width of any physical

> address used by the VM. The IPA_Bits is encoded in bits[7-0] of the

> machine type identifier.

>

> e.g, to configure a guest to use 48bit physical address size::

>

> vm_fd = ioctl(dev_fd, KVM_CREATE_VM, KVM_VM_TYPE_ARM_IPA_SIZE(48));

>

> The requested size (IPA_Bits) must be:

>

> == =========================================================

> 0 Implies default size, 40bits (for backward compatibility)

> N Implies N bits, where N is a positive integer such that,

> 32 <= N <= Host_IPA_Limit

> == =========================================================

> Host_IPA_Limit is the maximum possible value for IPA_Bits on the host

> and is dependent on the CPU capability and the kernel configuration.

> The limit can be retrieved using KVM_CAP_ARM_VM_IPA_SIZE of the

> KVM_CHECK_EXTENSION ioctl() at run-time.

>

> Creation of the VM will fail if the requested IPA size (whether it is

> implicit or explicit) is unsupported on the host.

https://docs.kernel.org/virt/kvm/api.html#kvm-create-vm

So if Host_IPA_Limit < 40, specifying 0 as the type will fail. This

actually confused libvirt, which uses "none" machine model to probe the

KVM availability, on M2 MacBook Air.

Fix this by using Host_IPA_Limit as the default type when

KVM_CAP_ARM_VM_IPA_SIZE is available.

Cc: qemu-stable@nongnu.org

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Message-id: 20230727073134.134102-3-akihiko.odaki@daynix.com

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

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

---

target/arm/kvm.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm.c

+++ b/target/arm/kvm.c

@@ -XXX,XX +XXX,XX @@ int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)

int kvm_arch_get_default_type(MachineState *ms)

{

- return 0;

+ bool fixed_ipa;

+ int size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa);

+ return fixed_ipa ? 0 : size;

}

int kvm_arch_init(MachineState *ms, KVMState *s)

--

2.34.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

On MIPS, QEMU requires KVM_VM_MIPS_VZ type for KVM. Report an error in

such a case as other architectures do when an error occurred during KVM

type decision.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Message-id: 20230727073134.134102-4-akihiko.odaki@daynix.com

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

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

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

---

target/mips/kvm.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/target/mips/kvm.c b/target/mips/kvm.c

index XXXXXXX..XXXXXXX 100644

--- a/target/mips/kvm.c

+++ b/target/mips/kvm.c

@@ -XXX,XX +XXX,XX @@ int kvm_arch_get_default_type(MachineState *machine)

}

#endif

+ error_report("KVM_VM_MIPS_VZ type is not available");

return -1;

}

--

2.34.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

On MIPS, kvm_arch_get_default_type() returns a negative value when an

error occurred so handle the case. Also, let other machines return

negative values when errors occur and declare returning a negative

value as the correct way to propagate an error that happened when

determining KVM type.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Message-id: 20230727073134.134102-5-akihiko.odaki@daynix.com

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

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

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

---

accel/kvm/kvm-all.c | 5 +++++

hw/arm/virt.c | 2 +-

hw/ppc/spapr.c | 2 +-

3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c

index XXXXXXX..XXXXXXX 100644

--- a/accel/kvm/kvm-all.c

+++ b/accel/kvm/kvm-all.c

@@ -XXX,XX +XXX,XX @@ static int kvm_init(MachineState *ms)

type = kvm_arch_get_default_type(ms);

}

+ if (type < 0) {

+ ret = -EINVAL;

+ goto err;

+ }

+

do {

ret = kvm_ioctl(s, KVM_CREATE_VM, type);

} while (ret == -EINTR);

diff --git a/hw/arm/virt.c b/hw/arm/virt.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/virt.c

+++ b/hw/arm/virt.c

@@ -XXX,XX +XXX,XX @@ static int virt_kvm_type(MachineState *ms, const char *type_str)

"require an IPA range (%d bits) larger than "

"the one supported by the host (%d bits)",

requested_pa_size, max_vm_pa_size);

- exit(1);

+ return -1;

}

/*

* We return the requested PA log size, unless KVM only supports

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/ppc/spapr.c

+++ b/hw/ppc/spapr.c

@@ -XXX,XX +XXX,XX @@ static int spapr_kvm_type(MachineState *machine, const char *vm_type)

}

error_report("Unknown kvm-type specified '%s'", vm_type);

- exit(1);

+ return -1;

}

/*

--

2.34.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

An error may occur after s->as is allocated, for example if the

KVM_CREATE_VM ioctl call fails.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Message-id: 20230727073134.134102-6-akihiko.odaki@daynix.com

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

[PMM: tweaked commit message]

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

---

accel/kvm/kvm-all.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c

index XXXXXXX..XXXXXXX 100644

--- a/accel/kvm/kvm-all.c

+++ b/accel/kvm/kvm-all.c

@@ -XXX,XX +XXX,XX @@ err:

if (s->fd != -1) {

close(s->fd);

}

+ g_free(s->as);

g_free(s->memory_listener.slots);

return ret;

--

2.34.1

From: Akihiko Odaki <akihiko.odaki@daynix.com>

The returned value was always zero and had no meaning.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Message-id: 20230727073134.134102-7-akihiko.odaki@daynix.com

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

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

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

---

accel/kvm/kvm-all.c | 9 ++-------

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

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c

index XXXXXXX..XXXXXXX 100644

--- a/accel/kvm/kvm-all.c

+++ b/accel/kvm/kvm-all.c

@@ -XXX,XX +XXX,XX @@ static void *kvm_dirty_ring_reaper_thread(void *data)

return NULL;

}

-static int kvm_dirty_ring_reaper_init(KVMState *s)

+static void kvm_dirty_ring_reaper_init(KVMState *s)

{

struct KVMDirtyRingReaper *r = &s->reaper;

qemu_thread_create(&r->reaper_thr, "kvm-reaper",

kvm_dirty_ring_reaper_thread,

s, QEMU_THREAD_JOINABLE);

-

- return 0;

}

static int kvm_dirty_ring_init(KVMState *s)

@@ -XXX,XX +XXX,XX @@ static int kvm_init(MachineState *ms)

}

if (s->kvm_dirty_ring_size) {

- ret = kvm_dirty_ring_reaper_init(s);

- if (ret) {

- goto err;

- }

+ kvm_dirty_ring_reaper_init(s);

}

if (kvm_check_extension(kvm_state, KVM_CAP_BINARY_STATS_FD)) {

--

2.34.1

For an Unsupported Atomic Update fault where the stage 1 translation

table descriptor update can't be done because it's to an unsupported

memory type, this is a stage 1 abort (per the Arm ARM R_VSXXT). This

means we should not set fi->s1ptw, because this will cause the code

in the get_phys_addr_lpae() error-exit path to mark it as stage 2.

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

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

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

---

target/arm/ptw.c | 1 -

1 file changed, 1 deletion(-)

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val,

if (unlikely(!host)) {

fi->type = ARMFault_UnsuppAtomicUpdate;

- fi->s1ptw = true;

return 0;

}

--

2.34.1

In S1_ptw_translate() we set up the ARMMMUFaultInfo if the attempt to

translate the page descriptor address into a physical address fails.

This used to only be possible if we are doing a stage 2 ptw for that

descriptor address, and so the code always sets fi->stage2 and

fi->s1ptw to true. However, with FEAT_RME it is also possible for

the lookup of the page descriptor address to fail because of a

Granule Protection Check fault. These should not be reported as

stage 2, otherwise arm_deliver_fault() will incorrectly set

HPFAR_EL2. Similarly the s1ptw bit should only be set for stage 2

faults on stage 1 translation table walks, i.e. not for GPC faults.

Add a comment to the the other place where we might detect a

stage2-fault-on-stage-1-ptw, in arm_casq_ptw(), noting why we know in

that case that it must really be a stage 2 fault and not a GPC fault.

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

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

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

---

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

1 file changed, 8 insertions(+), 2 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

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

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

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

fi->type = ARMFault_GPCFOnWalk;

}

fi->s2addr = addr;

- fi->stage2 = true;

- fi->s1ptw = true;

+ fi->stage2 = regime_is_stage2(s2_mmu_idx);

+ fi->s1ptw = fi->stage2;

fi->s1ns = !is_secure;

return false;

}

@@ -XXX,XX +XXX,XX @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t old_val,

env->tlb_fi = NULL;

if (unlikely(flags & TLB_INVALID_MASK)) {

+ /*

+ * We know this must be a stage 2 fault because the granule

+ * protection table does not separately track read and write

+ * permission, so all GPC faults are caught in S1_ptw_translate():

+ * we only get here for "readable but not writeable".

+ */

assert(fi->type != ARMFault_None);

fi->s2addr = ptw->out_virt;

fi->stage2 = true;

--

2.34.1

Message-id: 20230807141514.19075-4-peter.maydell@linaro.org

In commit 6d2654ffacea813916176 we created the S1Translate struct and

used it to plumb through various arguments that we were previously

passing one-at-a-time to get_phys_addr_v5(), get_phys_addr_v6(), and

get_phys_addr_lpae(). Extend that pattern to get_phys_addr_pmsav5(),

get_phys_addr_pmsav7(), get_phys_addr_pmsav8() and

get_phys_addr_disabled(), so that all the get_phys_addr_* functions

we call from get_phys_addr_nogpc() take the S1Translate struct rather

than the mmu_idx and is_secure bool.

(This refactoring is a prelude to having the called functions look

at ptw->is_space rather than using an is_secure boolean.)

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

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

Message-id: 20230807141514.19075-5-peter.maydell@linaro.org

---

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

1 file changed, 36 insertions(+), 21 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

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

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

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

return true;

}

-static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address,

- MMUAccessType access_type, ARMMMUIdx mmu_idx,

- bool is_secure, GetPhysAddrResult *result,

+static bool get_phys_addr_pmsav5(CPUARMState *env,

+ S1Translate *ptw,

+ uint32_t address,

+ MMUAccessType access_type,

+ GetPhysAddrResult *result,

ARMMMUFaultInfo *fi)

{

int n;

uint32_t mask;

uint32_t base;

+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;

bool is_user = regime_is_user(env, mmu_idx);

+ bool is_secure = arm_space_is_secure(ptw->in_space);

if (regime_translation_disabled(env, mmu_idx, is_secure)) {

/* MPU disabled. */

@@ -XXX,XX +XXX,XX @@ static bool pmsav7_use_background_region(ARMCPU *cpu, ARMMMUIdx mmu_idx,

return regime_sctlr(env, mmu_idx) & SCTLR_BR;

}

-static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,

- MMUAccessType access_type, ARMMMUIdx mmu_idx,

- bool secure, GetPhysAddrResult *result,

+static bool get_phys_addr_pmsav7(CPUARMState *env,

+ S1Translate *ptw,

+ uint32_t address,

+ MMUAccessType access_type,

+ GetPhysAddrResult *result,

ARMMMUFaultInfo *fi)

{

ARMCPU *cpu = env_archcpu(env);

int n;

+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;

bool is_user = regime_is_user(env, mmu_idx);

+ bool secure = arm_space_is_secure(ptw->in_space);

result->f.phys_addr = address;

result->f.lg_page_size = TARGET_PAGE_BITS;

@@ -XXX,XX +XXX,XX @@ void v8m_security_lookup(CPUARMState *env, uint32_t address,

}

}

-static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,

- MMUAccessType access_type, ARMMMUIdx mmu_idx,

- bool secure, GetPhysAddrResult *result,

+static bool get_phys_addr_pmsav8(CPUARMState *env,

+ S1Translate *ptw,

+ uint32_t address,

+ MMUAccessType access_type,

+ GetPhysAddrResult *result,

ARMMMUFaultInfo *fi)

{

V8M_SAttributes sattrs = {};

+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;

+ bool secure = arm_space_is_secure(ptw->in_space);

bool ret;

if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {

@@ -XXX,XX +XXX,XX @@ static ARMCacheAttrs combine_cacheattrs(uint64_t hcr,

* MMU disabled. S1 addresses within aa64 translation regimes are

* still checked for bounds -- see AArch64.S1DisabledOutput().

*/

-static bool get_phys_addr_disabled(CPUARMState *env, target_ulong address,

+static bool get_phys_addr_disabled(CPUARMState *env,

+ S1Translate *ptw,

+ target_ulong address,

MMUAccessType access_type,

- ARMMMUIdx mmu_idx, bool is_secure,

GetPhysAddrResult *result,

ARMMMUFaultInfo *fi)

{

+ ARMMMUIdx mmu_idx = ptw->in_mmu_idx;

+ bool is_secure = arm_space_is_secure(ptw->in_space);

uint8_t memattr = 0x00; /* Device nGnRnE */

uint8_t shareability = 0; /* non-shareable */

int r_el;

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

case ARMMMUIdx_Phys_Root:

case ARMMMUIdx_Phys_Realm:

/* Checking Phys early avoids special casing later vs regime_el. */

- return get_phys_addr_disabled(env, address, access_type, mmu_idx,

- is_secure, result, fi);

+ return get_phys_addr_disabled(env, ptw, address, access_type,

+ result, fi);

case ARMMMUIdx_Stage1_E0:

case ARMMMUIdx_Stage1_E1:

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

if (arm_feature(env, ARM_FEATURE_V8)) {

/* PMSAv8 */

- ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx,

- is_secure, result, fi);

+ ret = get_phys_addr_pmsav8(env, ptw, address, access_type,

+ result, fi);

} else if (arm_feature(env, ARM_FEATURE_V7)) {

/* PMSAv7 */

- ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,

- is_secure, result, fi);

+ ret = get_phys_addr_pmsav7(env, ptw, address, access_type,

+ result, fi);

} else {

/* Pre-v7 MPU */

- ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx,

- is_secure, result, fi);

+ ret = get_phys_addr_pmsav5(env, ptw, address, access_type,

+ result, fi);

}

qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32

" mmu_idx %u -> %s (prot %c%c%c)\n",

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

/* Definitely a real MMU, not an MPU */

if (regime_translation_disabled(env, mmu_idx, is_secure)) {

- return get_phys_addr_disabled(env, address, access_type, mmu_idx,

- is_secure, result, fi);

+ return get_phys_addr_disabled(env, ptw, address, access_type,

+ result, fi);

}

if (regime_using_lpae_format(env, mmu_idx)) {

--

2.34.1

Plumb the ARMSecurityState through to regime_translation_disabled()

rather than just a bool is_secure.

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

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