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>

kvm_arch_get_default_type() returns the default KVM type. This hook is

particularly useful to derive a KVM type that is valid for "none"

machine model, which is used by libvirt to probe the availability of

KVM.

For MIPS, the existing mips_kvm_type() is reused. This function ensures

the availability of VZ which is mandatory to use KVM on the current

QEMU.

Cc: qemu-stable@nongnu.org

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

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

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

[PMM: added doc comment for new function]

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

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

---

include/sysemu/kvm.h | 2 ++

target/mips/kvm_mips.h | 9 ---------

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

hw/mips/loongson3_virt.c | 2 --

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

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

target/mips/kvm.c | 2 +-

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

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

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

10 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h

index XXXXXXX..XXXXXXX 100644

--- a/include/sysemu/kvm.h

+++ b/include/sysemu/kvm.h

@@ -XXX,XX +XXX,XX @@ int kvm_arch_get_registers(CPUState *cpu);

int kvm_arch_put_registers(CPUState *cpu, int level);

+int kvm_arch_get_default_type(MachineState *ms);

+

int kvm_arch_init(MachineState *ms, KVMState *s);

int kvm_arch_init_vcpu(CPUState *cpu);

diff --git a/target/mips/kvm_mips.h b/target/mips/kvm_mips.h

index XXXXXXX..XXXXXXX 100644

--- a/target/mips/kvm_mips.h

+++ b/target/mips/kvm_mips.h

@@ -XXX,XX +XXX,XX @@ void kvm_mips_reset_vcpu(MIPSCPU *cpu);

int kvm_mips_set_interrupt(MIPSCPU *cpu, int irq, int level);

int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int level);

-#ifdef CONFIG_KVM

-int mips_kvm_type(MachineState *machine, const char *vm_type);

-#else

-static inline int mips_kvm_type(MachineState *machine, const char *vm_type)

-{

- return 0;

-}

-#endif

-

#endif /* KVM_MIPS_H */

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)

KVMState *s;

const KVMCapabilityInfo *missing_cap;

int ret;

- int type = 0;

+ int type;

uint64_t dirty_log_manual_caps;

qemu_mutex_init(&kml_slots_lock);

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

type = mc->kvm_type(ms, kvm_type);

} else if (mc->kvm_type) {

type = mc->kvm_type(ms, NULL);

+ } else {

+ type = kvm_arch_get_default_type(ms);

}

do {

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/mips/loongson3_virt.c

+++ b/hw/mips/loongson3_virt.c

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

#include "qemu/datadir.h"

#include "qapi/error.h"

#include "elf.h"

-#include "kvm_mips.h"

#include "hw/char/serial.h"

#include "hw/intc/loongson_liointc.h"

#include "hw/mips/mips.h"

@@ -XXX,XX +XXX,XX @@ static void loongson3v_machine_class_init(ObjectClass *oc, void *data)

mc->max_cpus = LOONGSON_MAX_VCPUS;

mc->default_ram_id = "loongson3.highram";

mc->default_ram_size = 1600 * MiB;

- mc->kvm_type = mips_kvm_type;

mc->minimum_page_bits = 14;

mc->default_nic = "virtio-net-pci";

}

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)

return ret > 0 ? ret : 40;

}

+int kvm_arch_get_default_type(MachineState *ms)

+{

+ return 0;

+}

+

int kvm_arch_init(MachineState *ms, KVMState *s)

{

int ret = 0;

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

index XXXXXXX..XXXXXXX 100644

--- a/target/i386/kvm/kvm.c

+++ b/target/i386/kvm/kvm.c

@@ -XXX,XX +XXX,XX @@ static void register_smram_listener(Notifier *n, void *unused)

&smram_address_space, 1, "kvm-smram");

}

+int kvm_arch_get_default_type(MachineState *ms)

+{

+ return 0;

+}

+

int kvm_arch_init(MachineState *ms, KVMState *s)

{

uint64_t identity_base = 0xfffbc000;

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_msi_data_to_gsi(uint32_t data)

abort();

}

-int mips_kvm_type(MachineState *machine, const char *vm_type)

+int kvm_arch_get_default_type(MachineState *machine)

{

#if defined(KVM_CAP_MIPS_VZ)

int r;

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

index XXXXXXX..XXXXXXX 100644

--- a/target/ppc/kvm.c

+++ b/target/ppc/kvm.c

@@ -XXX,XX +XXX,XX @@ static int kvm_ppc_register_host_cpu_type(void);

static void kvmppc_get_cpu_characteristics(KVMState *s);

static int kvmppc_get_dec_bits(void);

+int kvm_arch_get_default_type(MachineState *ms)

+{

+ return 0;

+}

+

int kvm_arch_init(MachineState *ms, KVMState *s)

{

cap_interrupt_unset = kvm_check_extension(s, KVM_CAP_PPC_UNSET_IRQ);

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

index XXXXXXX..XXXXXXX 100644

--- a/target/riscv/kvm.c

+++ b/target/riscv/kvm.c

@@ -XXX,XX +XXX,XX @@ int kvm_arch_add_msi_route_post(struct kvm_irq_routing_entry *route,

return 0;

}

+int kvm_arch_get_default_type(MachineState *ms)

+{

+ return 0;

+}

+

int kvm_arch_init(MachineState *ms, KVMState *s)

{

return 0;

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

index XXXXXXX..XXXXXXX 100644

--- a/target/s390x/kvm/kvm.c

+++ b/target/s390x/kvm/kvm.c

@@ -XXX,XX +XXX,XX @@ static void ccw_machine_class_foreach(ObjectClass *oc, void *opaque)

mc->default_cpu_type = S390_CPU_TYPE_NAME("host");

}

+int kvm_arch_get_default_type(MachineState *ms)

+{

+ return 0;

+}

+

int kvm_arch_init(MachineState *ms, KVMState *s)

{

object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE,

--

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

The s1ns bit in ARMMMUFaultInfo is documented as "true if

we faulted on a non-secure IPA while in secure state". Both the

places which look at this bit only do so after having confirmed

that this is a stage 2 fault and we're dealing with Secure EL2,

which leaves the ptw.c code free to set the bit to any random

value in the other cases.

Instead of taking advantage of that freedom, consistently

make the bit be set to false for the "not a stage 2 fault

for Secure EL2" cases. This removes some cases where we

were using an 'is_secure' boolean and leaving the reader

guessing about whether that was the right thing for Realm

and Root cases.

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

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

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

---

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

1 file changed, 15 insertions(+), 4 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 ARMSecuritySpace S2_security_space(ARMSecuritySpace s1_space,

}

}

+static bool fault_s1ns(ARMSecuritySpace space, ARMMMUIdx s2_mmu_idx)

+{

+ /*

+ * For stage 2 faults in Secure EL22, S1NS indicates

+ * whether the faulting IPA is in the Secure or NonSecure

+ * IPA space. For all other kinds of fault, it is false.

+ */

+ return space == ARMSS_Secure && regime_is_stage2(s2_mmu_idx)

+ && s2_mmu_idx == ARMMMUIdx_Stage2_S;

+}

+

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

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

hwaddr addr, ARMMMUFaultInfo *fi)

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

fi->s2addr = addr;

fi->stage2 = true;

fi->s1ptw = true;

- fi->s1ns = !is_secure;

+ fi->s1ns = fault_s1ns(ptw->in_space, s2_mmu_idx);

return false;

}

}

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

fi->s2addr = addr;

fi->stage2 = regime_is_stage2(s2_mmu_idx);

fi->s1ptw = fi->stage2;

- fi->s1ns = !is_secure;

+ fi->s1ns = fault_s1ns(ptw->in_space, s2_mmu_idx);

return false;

}

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

fi->s2addr = ptw->out_virt;

fi->stage2 = true;

fi->s1ptw = true;

- fi->s1ns = !ptw->in_secure;

+ fi->s1ns = fault_s1ns(ptw->in_space, ptw->in_ptw_idx);

return 0;

}

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

fi->level = level;

/* Tag the error as S2 for failed S1 PTW at S2 or ordinary S2. */

fi->stage2 = fi->s1ptw || regime_is_stage2(mmu_idx);

- fi->s1ns = mmu_idx == ARMMMUIdx_Stage2;

+ fi->s1ns = fault_s1ns(ptw->in_space, mmu_idx);

return true;

}

--

2.34.1

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 */