Re: [PATCH v2 10/16] migration: Don't abuse qemu_file transferred for RDMA
On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> Just create a variable for it, the same way that multifd does. This
> way it is safe to use for other thread, etc, etc.
>
> Signed-off-by: Juan Quintela
> ---
> migration/migration-stats.h | 4
> migration/migration-stats.c | 5 +++--
> migration/rdma.c| 22 --
> migration/trace-events | 2 +-
> 4 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index 9568b5b473..2e3e894307 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -89,6 +89,10 @@ typedef struct {
> * Maximum amount of data we can send in a cycle.
> */
> Stat64 rate_limit_max;
> +/*
> + * Number of bytes sent through RDMA.
> + */
> +Stat64 rdma_bytes;
> /*
> * How long has the setup stage took.
> */
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index abf2d38b18..4d8e9f93b7 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -68,8 +68,9 @@ void migration_rate_reset(QEMUFile *f)
> uint64_t migration_transferred_bytes(QEMUFile *f)
> {
> uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
> +uint64_t rdma = stat64_get(&mig_stats.rdma_bytes);
> uint64_t qemu_file = qemu_file_transferred(f);
>
> -trace_migration_transferred_bytes(qemu_file, multifd);
> -return qemu_file + multifd;
> +trace_migration_transferred_bytes(qemu_file, multifd, rdma);
> +return qemu_file + multifd + rdma;
> }
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 2e4dcff1c9..074456f9df 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2122,9 +2122,18 @@ retry:
> return -EIO;
> }
>
> +/*
> + * TODO: Here we are sending something, but we are not
> + * accounting for anything transferred. The following is
> wrong:
> + *
> + * stat64_add(&mig_stats.rdma_bytes, sge.length);
> + *
> + * because we are using some kind of compression. I
> + * would think that head.len would be the more similar
> + * thing to a correct value.
> + */
> stat64_add(&mig_stats.zero_pages,
> sge.length / qemu_target_page_size());
> -
> return 1;
> }
>
> @@ -2232,8 +2241,17 @@ retry:
>
> set_bit(chunk, block->transit_bitmap);
> stat64_add(&mig_stats.normal_pages, sge.length /
> qemu_target_page_size());
> +/*
> + * We are adding to transferred the amount of data written, but no
> + * overhead at all. I will asume that RDMA is magicaly and don't
> + * need to transfer (at least) the addresses where it wants to
> + * write the pages. Here it looks like it should be something
> + * like:
> + * sizeof(send_wr) + sge.length
> + * but this being RDMA, who knows.
> + */
> +stat64_add(&mig_stats.rdma_bytes, sge.length);
> ram_transferred_add(sge.length);
> -qemu_file_credit_transfer(f, sge.length);
> rdma->total_writes++;
>
> return 0;
> diff --git a/migration/trace-events b/migration/trace-events
> index cdaef7a1ea..54ae5653fd 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -187,7 +187,7 @@ process_incoming_migration_co_postcopy_end_main(void) ""
> postcopy_preempt_enabled(bool value) "%d"
>
> # migration-stats
> -migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd) "qemu_file
> %" PRIu64 " multifd %" PRIu64
> +migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd, uint64_t
> rdma) "qemu_file %" PRIu64 " multifd %" PRIu64 " RDMA %" PRIu64
>
> # channel.c
> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p
> ioctype=%s"
Reviewed-by: Leonardo Bras
Re: [PATCH v2 09/16] migration: We don't need the field rate_limit_used anymore
On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> Since previous commit, we calculate how much data we have send with
> migration_transferred_bytes() so no need to maintain this counter and
> remember to always update it.
>
> Signed-off-by: Juan Quintela
> Reviewed-by: Cédric Le Goater
> ---
> migration/migration-stats.h | 14 --
> migration/migration-stats.c | 6 --
> migration/multifd.c | 1 -
> migration/qemu-file.c | 4
> 4 files changed, 25 deletions(-)
>
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index f1465c2ebe..9568b5b473 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -89,10 +89,6 @@ typedef struct {
> * Maximum amount of data we can send in a cycle.
> */
> Stat64 rate_limit_max;
> -/*
> - * Amount of data we have sent in the current cycle.
> - */
> -Stat64 rate_limit_used;
> /*
> * How long has the setup stage took.
> */
> @@ -119,16 +115,6 @@ extern MigrationAtomicStats mig_stats;
> */
> void migration_time_since(MigrationAtomicStats *stats, int64_t since);
>
> -/**
> - * migration_rate_account: Increase the number of bytes transferred.
> - *
> - * Report on a number of bytes the have been transferred that need to
> - * be applied to the rate limiting calcuations.
> - *
> - * @len: amount of bytes transferred
> - */
> -void migration_rate_account(uint64_t len);
> -
> /**
> * migration_rate_get: Get the maximum amount that can be transferred.
> *
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index da2bb69a15..abf2d38b18 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -62,15 +62,9 @@ void migration_rate_set(uint64_t limit)
>
> void migration_rate_reset(QEMUFile *f)
> {
> -stat64_set(&mig_stats.rate_limit_used, 0);
> stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes(f));
> }
>
> -void migration_rate_account(uint64_t len)
> -{
> -stat64_add(&mig_stats.rate_limit_used, len);
> -}
> -
> uint64_t migration_transferred_bytes(QEMUFile *f)
> {
> uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 5052091ce2..aabf9b6d98 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -431,7 +431,6 @@ static int multifd_send_pages(QEMUFile *f)
> multifd_send_state->pages = p->pages;
> p->pages = pages;
> transferred = ((uint64_t) pages->num) * p->page_size + p->packet_len;
> -migration_rate_account(transferred);
> qemu_mutex_unlock(&p->mutex);
> stat64_add(&mig_stats.transferred, transferred);
> stat64_add(&mig_stats.multifd_bytes, transferred);
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 9c67b52fe0..acc282654a 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -292,7 +292,6 @@ void qemu_fflush(QEMUFile *f)
> qemu_file_set_error_obj(f, -EIO, local_error);
> } else {
> uint64_t size = iov_size(f->iov, f->iovcnt);
> -migration_rate_account(size);
> f->total_transferred += size;
> }
>
> @@ -344,9 +343,6 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t
> block_offset,
> if (f->hooks && f->hooks->save_page) {
> int ret = f->hooks->save_page(f, block_offset,
>offset, size, bytes_sent);
> -if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
> -migration_rate_account(size);
> -}
>
> if (ret != RAM_SAVE_CONTROL_DELAYED &&
> ret != RAM_SAVE_CONTROL_NOT_SUPP) {
I reviewed this one together with 8/16.
It makes sense for me to squash them together, but anyway:
Reviewed-by: Leonardo Bras
Re: [PATCH v2 08/16] migration: Use migration_transferred_bytes() to calculate rate_limit
On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela
> Reviewed-by: Cédric Le Goater
> ---
> migration/migration-stats.h | 8 +++-
> migration/migration-stats.c | 7 +--
> migration/migration.c | 2 +-
> 3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index 91fda378d3..f1465c2ebe 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -81,6 +81,10 @@ typedef struct {
> * Number of bytes sent during precopy stage.
> */
> Stat64 precopy_bytes;
> +/*
> + * Amount of transferred data at the start of current cycle.
> + */
> +Stat64 rate_limit_start;
> /*
> * Maximum amount of data we can send in a cycle.
> */
> @@ -136,8 +140,10 @@ uint64_t migration_rate_get(void);
> * migration_rate_reset: Reset the rate limit counter.
> *
> * This is called when we know we start a new transfer cycle.
> + *
> + * @f: QEMUFile used for main migration channel
> */
> -void migration_rate_reset(void);
> +void migration_rate_reset(QEMUFile *f);
>
> /**
> * migration_rate_set: Set the maximum amount that can be transferred.
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index 301392d208..da2bb69a15 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -31,7 +31,9 @@ bool migration_rate_exceeded(QEMUFile *f)
> return true;
> }
>
> -uint64_t rate_limit_used = stat64_get(&mig_stats.rate_limit_used);
> +uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
> +uint64_t rate_limit_current = migration_transferred_bytes(f);
> +uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
> uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
So, IIUC, instead of updating mig_stats.rate_limit_used every time data is sent,
the idea is to 'reset' it to migration_transferred_bytes() at the beginning of a
cycle, and read migration_transferred_bytes() again for checking if the limit
was not crossed.
Its a nice change since there is no need to update 2 counters, when 1 is enough.
I think it would look nicer if squashed with 9/16, though. It would make it more
clear this is being added to replace migration_rate_account() strategy.
What do you think?
Other than that,
Reviewed-by: Leonardo Bras
>
> if (rate_limit_max == RATE_LIMIT_MAX) {
> @@ -58,9 +60,10 @@ void migration_rate_set(uint64_t limit)
> stat64_set(&mig_stats.rate_limit_max, limit / XFER_LIMIT_RATIO);
> }
>
> -void migration_rate_reset(void)
> +void migration_rate_reset(QEMUFile *f)
> {
> stat64_set(&mig_stats.rate_limit_used, 0);
> +stat64_set(&mig_stats.rate_limit_start, migration_transferred_bytes(f));
> }
>
> void migration_rate_account(uint64_t len)
> diff --git a/migration/migration.c b/migration/migration.c
> index 39ff538046..e48dd593ed 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2691,7 +2691,7 @@ static void migration_update_counters(MigrationState *s,
> stat64_get(&mig_stats.dirty_bytes_last_sync) / bandwidth;
> }
>
> -migration_rate_reset();
> +migration_rate_reset(s->to_dst_file);
>
> update_iteration_initial_status(s);
>
[PATCH] hw/mips: Improve the default USB settings in the loongson3-virt machine
It's possible to compile QEMU without the USB devices (e.g. when using
"--without-default-devices" as option for the "configure" script).
To be still able to run the loongson3-virt machine in default mode with
such a QEMU binary, we have to check here for the availability of the
devices first before instantiating them.
Signed-off-by: Thomas Huth
---
The alternative would be to use a "#ifdef CONFIG_USB_OHCI_PCI" etc.
... not sure what is nicer ... what do you think?
hw/mips/loongson3_virt.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 216812f660..a0afb17030 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -447,10 +447,14 @@ static inline void
loongson3_virt_devices_init(MachineState *machine,
pci_vga_init(pci_bus);
-if (defaults_enabled()) {
+if (defaults_enabled() && module_object_class_by_name("pci-ohci")) {
pci_create_simple(pci_bus, -1, "pci-ohci");
-usb_create_simple(usb_bus_find(-1), "usb-kbd");
-usb_create_simple(usb_bus_find(-1), "usb-tablet");
+if (module_object_class_by_name("usb-kbd")) {
+usb_create_simple(usb_bus_find(-1), "usb-kbd");
+}
+if (module_object_class_by_name("usb-tablet")) {
+usb_create_simple(usb_bus_find(-1), "usb-tablet");
+}
}
for (i = 0; i < nb_nics; i++) {
--
2.31.1
Re: [PATCH v3 1/1] hw/arm/aspeed:Add vpd data for Rainier machine
[ ... ] However, regarding Cédric's log above, a reboot is expected on the first boot of a fresh image when there's valid VPD available. For the first boot of a fresh image we configure the kernel with a minimal devicetree that allows us to read the VPD data. This determines the system we're actually on and configures an appropriate devicetree for subsequent boots. We then reboot to pick up the new devicetree. Yes. Then, the behavior looks correct under QEMU : https://www.kaod.org/qemu/aspeed/rainier/rainer.log Here are the services which still have with some issues : * clear-once.serviceloaded failed failed Clear one time boot overrides * ncsi-linkspeed@eth0.service loaded failed failed Set eth0 gigabit link speed * ncsi-linkspeed@eth1.service loaded failed failed Set eth1 gigabit link speed * obmc-flash-bios-init.service loaded failed failed Setup Host FW directories * system-vpd.serviceloaded failed failed System VPD Collection * trace-enable.service loaded failed failed Enable Linux trace events in the boot loader C.
Re: [PATCH v3] target: ppc: Use MSR_HVB bit to get the target endianness for memory dump
On 5/23/23 15:52, Cédric Le Goater wrote:
On 5/22/23 18:02, Narayana Murty N wrote:
Currently on PPC64 qemu always dumps the guest memory in
Big Endian (BE) format even though the guest running in Little Endian
The patch is surely correct. I have problems understanding the config
you are testing. PPC Book3s has multiple hypervisor implementations :
1. pHyp (AKA PowerVM)
2. OPAL/PowerNV (AKA Power KVM-HV)
3. OPAL/PowerNV/pSeries (AKA Power KVMHV-on-pSeries)
4. pHyp/pSeries (very recent implementation, I don't know how it is
referred to in the kernel)
I am leaving the KVM-PR implementation out of the discussions for
simplicity.
QEMU also supports emulation of 2. and 3. in two different machines
PowerNV and pseries, although running pseries guests under a PowerNV
machine is slow, so is running pseries guests under pseries.
Could you please describe your environment ?
Thanks,
C.
It had been tested target machine OPAL/PowerNV with big endian host os.
and also target OPAL/PowerNV/pSeries little endian host setup with qemu.
(LE) mode. So crash tool fails to load the dump as illustrated below:
Log :
$ virsh dump DOMAIN --memory-only dump.file
Domain 'DOMAIN' dumped to dump.file
$ crash vmlinux dump.file
crash 8.0.2-1.el9
WARNING: endian mismatch:
crash utility: little-endian
dump.file: big-endian
WARNING: machine type mismatch:
crash utility: PPC64
dump.file: (unknown)
crash: dump.file: not a supported file format
This happens because cpu_get_dump_info() passes cpu->env->has_hv_mode
to function ppc_interrupts_little_endian(), the cpu->env->has_hv_mode
always set for powerNV even though the guest is not running in hv mode.
The hv mode should be taken from msr_mask MSR_HVB bit
(cpu->env.msr_mask & MSR_HVB). This patch fixes the issue by passing
MSR_HVB value to ppc_interrupts_little_endian() in order to determine
the guest endianness.
The crash tool also expects guest kernel endianness should match the
endianness of the dump.
The patch was tested on POWER9 box booted with Linux as host in
following cases:
Host-Endianess Qemu-Target-Machine Qemu-Guest-Endianess
Qemu-Generated-Guest
Memory-Dump-Format
BE powernv LE KVM guest LE
BE powernv BE KVM guest BE
LE powernv LE KVM guest LE
LE powernv BE KVM guest BE
LE pseries KVM LE KVM guest LE
LE pseries TCG LE guest LE
Signed-off-by: Narayana Murty N
---
Changes since V2:
commit message modified as per feedbak from Nicholas Piggin.
Changes since V1:
https://lore.kernel.org/qemu-devel/20230420145055.10196-1-nnmli...@linux.ibm.com/
The approach to solve the issue was changed based on feedback from
Fabiano Rosas on patch V1.
---
target/ppc/arch_dump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index f58e6359d5..a8315659d9 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -237,7 +237,7 @@ int cpu_get_dump_info(ArchDumpInfo *info,
info->d_machine = PPC_ELF_MACHINE;
info->d_class = ELFCLASS;
- if (ppc_interrupts_little_endian(cpu, cpu->env.has_hv_mode)) {
+ if (ppc_interrupts_little_endian(cpu, !!(cpu->env.msr_mask &
MSR_HVB))) {
info->d_endian = ELFDATA2LSB;
} else {
info->d_endian = ELFDATA2MSB;
Re: New container build error: mountpoint does not exit
On Wed, May 24, 2023 at 01:09:30PM -0700, Richard Henderson wrote: > Hi Eldon, > > New this morning are some odd failures in the container build stage, e.g > > https://gitlab.com/qemu-project/qemu/-/jobs/4345796216#L235 > > cgroups: cgroup mountpoint does not exist: unknown > > There are several such failures in that pipeline. I've not seen this before, > nor was it > happening yesterday. > > Any ideas? Hi Richard, If I had to guess, it's probably related to some changes in Camille's patchset. My first guess is that it is related to https://gitlab.com/qemu-project/qemu/-/commit/5f63a67adb58478974b91f5e5c2b1222b5c7f2cc which downgrades docker-in-docker from latest to stable-dind. I'm not sure how the stable tags are treated with docker, but it does seem that those tags haven't been updated in a couple of years. This person on SO seems to have fixed a similar issue in gitlab ci by upgrading: https://stackoverflow.com/q/70707572 The cgroups error leads me to believe that the older dind service is expecting cgroups v1, while the hardware runner is running FCOS with cgroups v2. I ran a test with just that patch omitted on my repo, with better results: https://gitlab.com/eldondev/qemu/-/pipelines/878387857 I will see if the opensbi build which also uses stable-dind and failed in that pipeline is successful when attempting to use latest dind instead. If using stable is compelling, I can work on making the hardware runner support it. Thanks, Eldon
Re: [PATCH v7 08/14] KVM: Rename mmu_notifier_*
On 2023-05-24 22:33:36, Peter Zijlstra wrote:
> On Wed, May 24, 2023 at 01:16:03PM -0700, Sean Christopherson wrote:
>
> > Atomics aren't memory barriers on all architectures, e.g. see the various
> > definitions of smp_mb__after_atomic().
> >
> > Even if atomic operations did provide barriers, using an atomic would be
> > overkill
> > and a net negative. On strongly ordered architectures like x86, memory
> > barriers are
> > just compiler barriers, whereas atomics may be more expensive.
>
> Not quite, smp_{r,w}mb() and smp_mb__{before,after}_atomic() are
> compiler barriers on the TSO archs, but smp_mb() very much isn't. TSO
> still allows stores to be delayed vs later loads (iow it doesn't pretend
> to hide the store buffer).
>
> > Of course, the only
> > accesses outside of mmu_lock are reads, so on x86 that "atomic" access is
> > just a
> > READ_ONCE() load, but that's not the case for all architectures.
>
> This is true on *all* archs. atomic_set() and atomic_read() are no more
> and no less than WRITE_ONCE() / READ_ONCE().
>
> > Anyways, the point is that atomics and memory barriers are different things
> > that
> > serve different purposes.
>
> This is true; esp. on the weakly ordered architectures where atomics do
> not naturally imply any ordering.
Thanks for the information, everyone.
Re: [PATCH v2 07/16] migration: Add a trace for migration_transferred_bytes
On Mon, 2023-05-15 at 21:57 +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela
> Reviewed-by: Cédric Le Goater
> ---
> migration/migration-stats.c | 8 ++--
> migration/trace-events | 3 +++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index 9bd97caa23..301392d208 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -14,6 +14,7 @@
> #include "qemu/stats64.h"
> #include "qemu/timer.h"
> #include "qemu-file.h"
> +#include "trace.h"
> #include "migration-stats.h"
>
> MigrationAtomicStats mig_stats;
> @@ -69,6 +70,9 @@ void migration_rate_account(uint64_t len)
>
> uint64_t migration_transferred_bytes(QEMUFile *f)
> {
> -return qemu_file_transferred(f) + stat64_get(&mig_stats.multifd_bytes);
> -}
> +uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
> +uint64_t qemu_file = qemu_file_transferred(f);
>
> +trace_migration_transferred_bytes(qemu_file, multifd);
> +return qemu_file + multifd;
> +}
> diff --git a/migration/trace-events b/migration/trace-events
> index f39818c329..cdaef7a1ea 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -186,6 +186,9 @@ process_incoming_migration_co_end(int ret, int ps)
> "ret=%d postcopy-state=%d"
> process_incoming_migration_co_postcopy_end_main(void) ""
> postcopy_preempt_enabled(bool value) "%d"
>
> +# migration-stats
> +migration_transferred_bytes(uint64_t qemu_file, uint64_t multifd) "qemu_file
> %" PRIu64 " multifd %" PRIu64
> +
> # channel.c
> migration_set_incoming_channel(void *ioc, const char *ioctype) "ioc=%p
> ioctype=%s"
> migration_set_outgoing_channel(void *ioc, const char *ioctype, const char
> *hostname, void *err) "ioc=%p ioctype=%s hostname=%s err=%p"
LGTM
Reviewed-by: Leonardo Bras
Re: [PATCH v2 06/16] migration: Move migration_total_bytes() to migration-stats.c
On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> Once there rename it to migration_transferred_bytes() and pass a
> QEMUFile instead of a migration object.
>
> Signed-off-by: Juan Quintela
> Reviewed-by: Cédric Le Goater
> ---
> migration/migration-stats.h | 11 +++
> migration/migration-stats.c | 6 ++
> migration/migration.c | 13 +++--
> 3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index e39c083245..91fda378d3 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -147,4 +147,15 @@ void migration_rate_reset(void);
> * @new_rate: new maximum amount
> */
> void migration_rate_set(uint64_t new_rate);
> +
> +/**
> + * migration_transferred_bytes: Return number of bytes transferred
> + *
> + * @f: QEMUFile used for main migration channel
> + *
> + * Returns how many bytes have we transferred since the beginning of
> + * the migration. It accounts for bytes sent through any migration
> + * channel, multifd, qemu_file, rdma,
> + */
> +uint64_t migration_transferred_bytes(QEMUFile *f);
> #endif
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index 1b16edae7d..9bd97caa23 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -66,3 +66,9 @@ void migration_rate_account(uint64_t len)
> {
> stat64_add(&mig_stats.rate_limit_used, len);
> }
> +
> +uint64_t migration_transferred_bytes(QEMUFile *f)
> +{
> +return qemu_file_transferred(f) + stat64_get(&mig_stats.multifd_bytes);
> +}
> +
> diff --git a/migration/migration.c b/migration/migration.c
> index 594709dbbc..39ff538046 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2624,16 +2624,9 @@ static MigThrError
> migration_detect_error(MigrationState *s)
> }
> }
>
> -/* How many bytes have we transferred since the beginning of the migration */
> -static uint64_t migration_total_bytes(MigrationState *s)
> -{
> -return qemu_file_transferred(s->to_dst_file) +
> -stat64_get(&mig_stats.multifd_bytes);
> -}
> -
> static void migration_calculate_complete(MigrationState *s)
> {
> -uint64_t bytes = migration_total_bytes(s);
> +uint64_t bytes = migration_transferred_bytes(s->to_dst_file);
> int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> int64_t transfer_time;
>
> @@ -2659,7 +2652,7 @@ static void
> update_iteration_initial_status(MigrationState *s)
> * wrong speed calculation.
> */
> s->iteration_start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> -s->iteration_initial_bytes = migration_total_bytes(s);
> +s->iteration_initial_bytes = migration_transferred_bytes(s->to_dst_file);
> s->iteration_initial_pages = ram_get_total_transferred_pages();
> }
>
> @@ -2674,7 +2667,7 @@ static void migration_update_counters(MigrationState *s,
> return;
> }
>
> -current_bytes = migration_total_bytes(s);
> +current_bytes = migration_transferred_bytes(s->to_dst_file);
> transferred = current_bytes - s->iteration_initial_bytes;
> time_spent = current_time - s->iteration_start_time;
> bandwidth = (double)transferred / time_spent;
LGTM
Reviewed-by: Leonardo Bras
Re: [PATCH v2 05/16] migration: Move rate_limit_max and rate_limit_used to migration_stats
On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> These way we can make them atomic and use this functions from any
> place. I also moved all functions that use rate_limit to
> migration-stats.
>
> Functions got renamed, they are not qemu_file anymore.
>
> qemu_file_rate_limit -> migration_rate_exceeded
> qemu_file_set_rate_limit -> migration_rate_set
> qemu_file_get_rate_limit -> migration_rate_get
> qemu_file_reset_rate_limit -> migration_rate_reset
> qemu_file_acct_rate_limit -> migration_rate_account.
>
> Signed-off-by: Juan Quintela
> Reviewed-by: Harsh Prateek Bora
>
> ---
>
> s/this/these/ (harsh)
> If you have any good suggestion for better names, I am all ears.
> Fix missing / XFER_LIMIT_RATIO in migration_rate_set(quintela)
> ---
> include/migration/qemu-file-types.h | 12 ++-
> migration/migration-stats.h | 47 ++
> migration/options.h | 7
> migration/qemu-file.h | 11 --
> hw/ppc/spapr.c | 4 +--
> hw/s390x/s390-stattrib.c| 2 +-
> migration/block-dirty-bitmap.c | 2 +-
> migration/block.c | 5 +--
> migration/migration-stats.c | 44
> migration/migration.c | 14
> migration/multifd.c | 2 +-
> migration/options.c | 7 ++--
> migration/qemu-file.c | 52 ++---
> migration/ram.c | 2 +-
> migration/savevm.c | 2 +-
> 15 files changed, 124 insertions(+), 89 deletions(-)
>
> diff --git a/include/migration/qemu-file-types.h
> b/include/migration/qemu-file-types.h
> index 1436f9ce92..9ba163f333 100644
> --- a/include/migration/qemu-file-types.h
> +++ b/include/migration/qemu-file-types.h
> @@ -165,6 +165,16 @@ size_t coroutine_mixed_fn
> qemu_get_counted_string(QEMUFile *f, char buf[256]);
>
> void qemu_put_counted_string(QEMUFile *f, const char *name);
>
> -int qemu_file_rate_limit(QEMUFile *f);
> +/**
> + * migration_rate_exceeded: Check if we have exceeded rate for this interval
> + *
> + * Checks if we have already transferred more data that we are allowed
> + * in the current interval.
> + *
> + * @f: QEMUFile used for main migration channel
> + *
> + * Returns if we should stop sending data for this interval.
> + */
> +bool migration_rate_exceeded(QEMUFile *f);
>
> #endif
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index 21402af9e4..e39c083245 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -15,6 +15,12 @@
>
> #include "qemu/stats64.h"
>
> +/*
> + * Amount of time to allocate to each "chunk" of bandwidth-throttled
> + * data.
> + */
> +#define BUFFER_DELAY 100
> +
> /*
> * If rate_limit_max is 0, there is special code to remove the rate
> * limit.
> @@ -75,6 +81,14 @@ typedef struct {
> * Number of bytes sent during precopy stage.
> */
> Stat64 precopy_bytes;
> +/*
> + * Maximum amount of data we can send in a cycle.
> + */
> +Stat64 rate_limit_max;
> +/*
> + * Amount of data we have sent in the current cycle.
> + */
> +Stat64 rate_limit_used;
> /*
> * How long has the setup stage took.
> */
> @@ -100,4 +114,37 @@ extern MigrationAtomicStats mig_stats;
> * Returns: Nothing. The time is stored in val.
> */
> void migration_time_since(MigrationAtomicStats *stats, int64_t since);
> +
> +/**
> + * migration_rate_account: Increase the number of bytes transferred.
> + *
> + * Report on a number of bytes the have been transferred that need to
> + * be applied to the rate limiting calcuations.
s/calcuations/calculations
> + *
> + * @len: amount of bytes transferred
> + */
> +void migration_rate_account(uint64_t len);
> +
> +/**
> + * migration_rate_get: Get the maximum amount that can be transferred.
> + *
> + * Returns the maximum number of bytes that can be transferred in a cycle.
> + */
> +uint64_t migration_rate_get(void);
maybe migration_max_rate_get() ?
> +
> +/**
> + * migration_rate_reset: Reset the rate limit counter.
> + *
> + * This is called when we know we start a new transfer cycle.
> + */
> +void migration_rate_reset(void);
> +
> +/**
> + * migration_rate_set: Set the maximum amount that can be transferred.
> + *
> + * Sets the maximum amount of bytes that can be transferred in one cycle.
> + *
> + * @new_rate: new maximum amount
> + */
> +void migration_rate_set(uint64_t new_rate);
maybe migration_max_rate_set() ?
> #endif
> diff --git a/migration/options.h b/migration/options.h
> index 5cca3326d6..45991af3c2 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -17,13 +17,6 @@
> #include "hw/qdev-properties.h"
> #include "hw/qdev-properties-system.h"
>
> -/* constants */
> -
> -/* Amount of time to allocate to each "chunk" of bandwidth-throttled
> - * data. */
> -#define BUFFER_DELAY
Re: [PATCH v4 2/3] hw/riscv: sifive_e: Support the watchdog timer of HiFive 1 rev b.
On Tue, May 23, 2023 at 6:50 PM Tommy Wu wrote:
>
> Create the AON device when we realize the sifive_e machine.
> This patch only implemented the functionality of the watchdog timer,
> not all the functionality of the AON device.
>
> Signed-off-by: Tommy Wu
> Reviewed-by: Frank Chang
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/riscv/Kconfig| 1 +
> hw/riscv/sifive_e.c | 13 +++--
> include/hw/riscv/sifive_e.h | 8 +---
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 6528ebfa3a..b6a5eb4452 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -60,6 +60,7 @@ config SIFIVE_E
> select SIFIVE_PLIC
> select SIFIVE_UART
> select SIFIVE_E_PRCI
> +select SIFIVE_E_AON
> select UNIMP
>
> config SIFIVE_U
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 04939b60c3..731ed0e11d 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -45,6 +45,7 @@
> #include "hw/intc/riscv_aclint.h"
> #include "hw/intc/sifive_plic.h"
> #include "hw/misc/sifive_e_prci.h"
> +#include "hw/misc/sifive_e_aon.h"
> #include "chardev/char.h"
> #include "sysemu/sysemu.h"
>
> @@ -223,8 +224,13 @@ static void sifive_e_soc_realize(DeviceState *dev, Error
> **errp)
> RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus,
> RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME,
> RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, false);
> -create_unimplemented_device("riscv.sifive.e.aon",
> -memmap[SIFIVE_E_DEV_AON].base, memmap[SIFIVE_E_DEV_AON].size);
> +
> +s->aon = qdev_new(TYPE_SIFIVE_E_AON);
> +if (!sysbus_realize(SYS_BUS_DEVICE(s->aon), errp)) {
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(s->aon), 0,
> memmap[SIFIVE_E_DEV_AON].base);
> +
> sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base);
>
> /* GPIO */
> @@ -245,6 +251,9 @@ static void sifive_e_soc_realize(DeviceState *dev, Error
> **errp)
> qdev_get_gpio_in(DEVICE(s->plic),
> SIFIVE_E_GPIO0_IRQ0 + i));
> }
> +sysbus_connect_irq(SYS_BUS_DEVICE(s->aon), 0,
> + qdev_get_gpio_in(DEVICE(s->plic),
> +SIFIVE_E_AON_WDT_IRQ));
>
> sifive_uart_create(sys_mem, memmap[SIFIVE_E_DEV_UART0].base,
> serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_E_UART0_IRQ));
> diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
> index b824a79e2d..a094b47e0b 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -35,6 +35,7 @@ typedef struct SiFiveESoCState {
> /*< public >*/
> RISCVHartArrayState cpus;
> DeviceState *plic;
> +DeviceState *aon;
> SIFIVEGPIOState gpio;
> MemoryRegion xip_mem;
> MemoryRegion mask_rom;
> @@ -76,9 +77,10 @@ enum {
> };
>
> enum {
> -SIFIVE_E_UART0_IRQ = 3,
> -SIFIVE_E_UART1_IRQ = 4,
> -SIFIVE_E_GPIO0_IRQ0 = 8
> +SIFIVE_E_AON_WDT_IRQ = 1,
> +SIFIVE_E_UART0_IRQ= 3,
> +SIFIVE_E_UART1_IRQ= 4,
> +SIFIVE_E_GPIO0_IRQ0 = 8
> };
>
> #define SIFIVE_E_PLIC_HART_CONFIG "M"
> --
> 2.27.0
>
>
Re: [PATCH 1/2] target/riscv: Add a function to refresh the dynamic CSRs xml.
On Tue, May 23, 2023 at 9:46 PM Tommy Wu wrote:
>
> When we change the cpu extension state after the cpu is
> realized, we cannot print the value of some CSRs in the remote
> gdb debugger. The root cause is that the dynamic CSR xml is
> generated when the cpu is realized.
>
> This patch add a function to refresh the dynamic CSR xml after
> the cpu is realized.
>
> Signed-off-by: Tommy Wu
> Reviewed-by: Frank Chang
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/gdbstub.c | 12
> 2 files changed, 14 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index de7e43126a..dc8e592275 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -494,6 +494,7 @@ struct ArchCPU {
> CPUNegativeOffsetState neg;
> CPURISCVState env;
>
> +int dyn_csr_base_reg;
> char *dyn_csr_xml;
> char *dyn_vreg_xml;
>
> @@ -781,6 +782,7 @@ void riscv_get_csr_ops(int csrno, riscv_csr_operations
> *ops);
> void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
>
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
> +void riscv_refresh_dynamic_csr_xml(CPUState *cs);
>
> uint8_t satp_mode_max_from_map(uint32_t map);
> const char *satp_mode_str(uint8_t satp_mode, bool is_32_bit);
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 524bede865..9e97ee2c35 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -230,6 +230,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
> base_reg)
> bitsize = 64;
> }
>
> +cpu->dyn_csr_base_reg = base_reg;
> +
> g_string_printf(s, "");
> g_string_append_printf(s, " \"gdb-target.dtd\">");
> g_string_append_printf(s, "");
> @@ -349,3 +351,13 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> *cs)
> "riscv-csr.xml", 0);
> }
> }
> +
> +void riscv_refresh_dynamic_csr_xml(CPUState *cs)
> +{
> +RISCVCPU *cpu = RISCV_CPU(cs);
> +if (!cpu->dyn_csr_xml) {
> +g_assert_not_reached();
> +}
> +g_free(cpu->dyn_csr_xml);
> +riscv_gen_dynamic_csr_xml(cs, cpu->dyn_csr_base_reg);
I don't really understand why we need dyn_csr_base_reg, could we just
use cs->gdb_num_regs directly here?
Alistair
> +}
> --
> 2.38.1
>
>
Re: [PATCH v5] hw/riscv: qemu crash when NUMA nodes exceed available CPUs
On Fri, May 19, 2023 at 12:38 PM Yin Wang wrote:
>
> Command "qemu-system-riscv64 -machine virt
> -m 2G -smp 1 -numa node,mem=1G -numa node,mem=1G"
> would trigger this problem.Backtrace with:
> #0 0x55b5b1a4 in riscv_numa_get_default_cpu_node_id at
> ../hw/riscv/numa.c:211
> #1 0x558ce510 in machine_numa_finish_cpu_init at
> ../hw/core/machine.c:1230
> #2 0x558ce9d3 in machine_run_board_init at
> ../hw/core/machine.c:1346
> #3 0x55aaedc3 in qemu_init_board at ../softmmu/vl.c:2513
> #4 0x55aaf064 in qmp_x_exit_preconfig at ../softmmu/vl.c:2609
> #5 0x55ab1916 in qemu_init at ../softmmu/vl.c:3617
> #6 0x5585463b in main at ../softmmu/main.c:47
> This commit fixes the issue by adding parameter checks.
>
> Reviewed-by: Alistair Francis
> Reviewed-by: Daniel Henrique Barboza
> Reviewed-by: LIU Zhiwei
> Reviewed-by: Weiwei Li
> Signed-off-by: Yin Wang
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> hw/riscv/numa.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/riscv/numa.c b/hw/riscv/numa.c
> index 4720102561..e0414d5b1b 100644
> --- a/hw/riscv/numa.c
> +++ b/hw/riscv/numa.c
> @@ -207,6 +207,12 @@ int64_t riscv_numa_get_default_cpu_node_id(const
> MachineState *ms, int idx)
> {
> int64_t nidx = 0;
>
> +if (ms->numa_state->num_nodes > ms->smp.cpus) {
> +error_report("Number of NUMA nodes (%d)"
> + " cannot exceed the number of available CPUs (%d).",
> + ms->numa_state->num_nodes, ms->smp.max_cpus);
> +exit(EXIT_FAILURE);
> +}
> if (ms->numa_state->num_nodes) {
> nidx = idx / (ms->smp.cpus / ms->numa_state->num_nodes);
> if (ms->numa_state->num_nodes <= nidx) {
> --
> 2.34.1
>
>
Re: [PATCH 0/5] hw/riscv/opentitan: Correct QOM type/size of OpenTitanState
On Sat, May 20, 2023 at 3:47 PM Philippe Mathieu-Daudé wrote: > > Hi, > > This series fix a QOM issue with the OpenTitanState > structure, noticed while auditing QOM relations globally. > > All patches are trivial to review. > > Regards, > > Phil. > > Philippe Mathieu-Daudé (5): > hw/riscv/opentitan: Rename machine_[class]_init() functions > hw/riscv/opentitan: Declare QOM types using DEFINE_TYPES() macro > hw/riscv/opentitan: Add TYPE_OPENTITAN_MACHINE definition > hw/riscv/opentitan: Explicit machine type definition > hw/riscv/opentitan: Correct OpenTitanState parent type/size Thanks! Applied to riscv-to-apply.next Alistair > > include/hw/riscv/opentitan.h | 6 +- > hw/riscv/opentitan.c | 38 +++- > 2 files changed, 25 insertions(+), 19 deletions(-) > > -- > 2.38.1 > >
Re: [PATCH 5/5] hw/riscv/opentitan: Correct OpenTitanState parent type/size
On Sat, May 20, 2023 at 3:47 PM Philippe Mathieu-Daudé
wrote:
>
> OpenTitanState is the 'machine' (or 'board') state: it isn't
> a SysBus device, but inherits from the MachineState type.
> Correct the instance size.
> Doing so we avoid leaking an OpenTitanState pointer in
> opentitan_machine_init().
>
> Fixes: fe0fe4735e ("riscv: Initial commit of OpenTitan machine")
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> include/hw/riscv/opentitan.h | 3 ++-
> hw/riscv/opentitan.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
> index 806ff73528..609473d07b 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -55,10 +55,11 @@ struct LowRISCIbexSoCState {
> };
>
> #define TYPE_OPENTITAN_MACHINE MACHINE_TYPE_NAME("opentitan")
> +OBJECT_DECLARE_SIMPLE_TYPE(OpenTitanState, OPENTITAN_MACHINE)
>
> typedef struct OpenTitanState {
> /*< private >*/
> -SysBusDevice parent_obj;
> +MachineState parent_obj;
>
> /*< public >*/
> LowRISCIbexSoCState soc;
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 9535308197..6a2fcc4ade 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -78,8 +78,8 @@ static const MemMapEntry ibex_memmap[] = {
> static void opentitan_machine_init(MachineState *machine)
> {
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> +OpenTitanState *s = OPENTITAN_MACHINE(machine);
> const MemMapEntry *memmap = ibex_memmap;
> -OpenTitanState *s = g_new0(OpenTitanState, 1);
> MemoryRegion *sys_mem = get_system_memory();
>
> if (machine->ram_size != mc->default_ram_size) {
> @@ -330,6 +330,7 @@ static const TypeInfo open_titan_types[] = {
> }, {
> .name = TYPE_OPENTITAN_MACHINE,
> .parent = TYPE_MACHINE,
> +.instance_size = sizeof(OpenTitanState),
> .class_init = opentitan_machine_class_init,
> }
> };
> --
> 2.38.1
>
>
Re: [PATCH 4/5] hw/riscv/opentitan: Explicit machine type definition
On Sat, May 20, 2023 at 3:47 PM Philippe Mathieu-Daudé
wrote:
>
> Expand the DEFINE_MACHINE() macro, converting the class_init()
> handler.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> include/hw/riscv/opentitan.h | 3 ++-
> hw/riscv/opentitan.c | 10 +++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
> index fd70226ed8..806ff73528 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -24,6 +24,7 @@
> #include "hw/char/ibex_uart.h"
> #include "hw/timer/ibex_timer.h"
> #include "hw/ssi/ibex_spi_host.h"
> +#include "hw/boards.h"
> #include "qom/object.h"
>
> #define TYPE_RISCV_IBEX_SOC "riscv.lowrisc.ibex.soc"
> @@ -53,7 +54,7 @@ struct LowRISCIbexSoCState {
> MemoryRegion flash_alias;
> };
>
> -#define TYPE_OPENTITAN_MACHINE "opentitan"
> +#define TYPE_OPENTITAN_MACHINE MACHINE_TYPE_NAME("opentitan")
>
> typedef struct OpenTitanState {
> /*< private >*/
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 7d7159ea30..9535308197 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -108,8 +108,10 @@ static void opentitan_machine_init(MachineState *machine)
> }
> }
>
> -static void opentitan_machine_class_init(MachineClass *mc)
> +static void opentitan_machine_class_init(ObjectClass *oc, void *data)
> {
> +MachineClass *mc = MACHINE_CLASS(oc);
> +
> mc->desc = "RISC-V Board compatible with OpenTitan";
> mc->init = opentitan_machine_init;
> mc->max_cpus = 1;
> @@ -118,8 +120,6 @@ static void opentitan_machine_class_init(MachineClass *mc)
> mc->default_ram_size = ibex_memmap[IBEX_DEV_RAM].size;
> }
>
> -DEFINE_MACHINE(TYPE_OPENTITAN_MACHINE, opentitan_machine_class_init)
> -
> static void lowrisc_ibex_soc_init(Object *obj)
> {
> LowRISCIbexSoCState *s = RISCV_IBEX_SOC(obj);
> @@ -327,6 +327,10 @@ static const TypeInfo open_titan_types[] = {
> .instance_size = sizeof(LowRISCIbexSoCState),
> .instance_init = lowrisc_ibex_soc_init,
> .class_init = lowrisc_ibex_soc_class_init,
> +}, {
> +.name = TYPE_OPENTITAN_MACHINE,
> +.parent = TYPE_MACHINE,
> +.class_init = opentitan_machine_class_init,
> }
> };
>
> --
> 2.38.1
>
>
Re: [PATCH 3/5] hw/riscv/opentitan: Add TYPE_OPENTITAN_MACHINE definition
On Sat, May 20, 2023 at 3:46 PM Philippe Mathieu-Daudé
wrote:
>
> QOM type names are usually defined as TYPE_FOO.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> include/hw/riscv/opentitan.h | 2 ++
> hw/riscv/opentitan.c | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
> index c40b05052a..fd70226ed8 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -53,6 +53,8 @@ struct LowRISCIbexSoCState {
> MemoryRegion flash_alias;
> };
>
> +#define TYPE_OPENTITAN_MACHINE "opentitan"
> +
> typedef struct OpenTitanState {
> /*< private >*/
> SysBusDevice parent_obj;
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 294955eeea..7d7159ea30 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -118,7 +118,7 @@ static void opentitan_machine_class_init(MachineClass *mc)
> mc->default_ram_size = ibex_memmap[IBEX_DEV_RAM].size;
> }
>
> -DEFINE_MACHINE("opentitan", opentitan_machine_class_init)
> +DEFINE_MACHINE(TYPE_OPENTITAN_MACHINE, opentitan_machine_class_init)
>
> static void lowrisc_ibex_soc_init(Object *obj)
> {
> --
> 2.38.1
>
>
Re: [PATCH 2/5] hw/riscv/opentitan: Declare QOM types using DEFINE_TYPES() macro
On Sat, May 20, 2023 at 3:46 PM Philippe Mathieu-Daudé
wrote:
>
> When multiple QOM types are registered in the same file,
> it is simpler to use the the DEFINE_TYPES() macro. Replace
> the type_init() / type_register_static() combination. This
> is in preparation of adding the OpenTitan machine type to
> this array in a pair of commits.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/riscv/opentitan.c | 21 +
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 2d21ee39c5..294955eeea 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -320,17 +320,14 @@ static void lowrisc_ibex_soc_class_init(ObjectClass
> *oc, void *data)
> dc->user_creatable = false;
> }
>
> -static const TypeInfo lowrisc_ibex_soc_type_info = {
> -.name = TYPE_RISCV_IBEX_SOC,
> -.parent = TYPE_DEVICE,
> -.instance_size = sizeof(LowRISCIbexSoCState),
> -.instance_init = lowrisc_ibex_soc_init,
> -.class_init = lowrisc_ibex_soc_class_init,
> +static const TypeInfo open_titan_types[] = {
> +{
> +.name = TYPE_RISCV_IBEX_SOC,
> +.parent = TYPE_DEVICE,
> +.instance_size = sizeof(LowRISCIbexSoCState),
> +.instance_init = lowrisc_ibex_soc_init,
> +.class_init = lowrisc_ibex_soc_class_init,
> +}
> };
>
> -static void lowrisc_ibex_soc_register_types(void)
> -{
> -type_register_static(&lowrisc_ibex_soc_type_info);
> -}
> -
> -type_init(lowrisc_ibex_soc_register_types)
> +DEFINE_TYPES(open_titan_types)
> --
> 2.38.1
>
>
Re: [PATCH 1/5] hw/riscv/opentitan: Rename machine_[class]_init() functions
On Sat, May 20, 2023 at 3:47 PM Philippe Mathieu-Daudé
wrote:
>
> Follow QOM style which declares FOO_init() as instance
> initializer and FOO_class_init() as class initializer:
> rename the OpenTitan machine class/instance init()
> accordingly.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/riscv/opentitan.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index bc678766e7..2d21ee39c5 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -75,7 +75,7 @@ static const MemMapEntry ibex_memmap[] = {
> [IBEX_DEV_FLASH_VIRTUAL] = { 0x8000, 0x8 },
> };
>
> -static void opentitan_board_init(MachineState *machine)
> +static void opentitan_machine_init(MachineState *machine)
> {
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> const MemMapEntry *memmap = ibex_memmap;
> @@ -108,17 +108,17 @@ static void opentitan_board_init(MachineState *machine)
> }
> }
>
> -static void opentitan_machine_init(MachineClass *mc)
> +static void opentitan_machine_class_init(MachineClass *mc)
> {
> mc->desc = "RISC-V Board compatible with OpenTitan";
> -mc->init = opentitan_board_init;
> +mc->init = opentitan_machine_init;
> mc->max_cpus = 1;
> mc->default_cpu_type = TYPE_RISCV_CPU_IBEX;
> mc->default_ram_id = "riscv.lowrisc.ibex.ram";
> mc->default_ram_size = ibex_memmap[IBEX_DEV_RAM].size;
> }
>
> -DEFINE_MACHINE("opentitan", opentitan_machine_init)
> +DEFINE_MACHINE("opentitan", opentitan_machine_class_init)
>
> static void lowrisc_ibex_soc_init(Object *obj)
> {
> --
> 2.38.1
>
>
Re: [PATCH v2 04/16] qemu-file: Account for rate_limit usage on qemu_fflush()
On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> That is the moment we know we have transferred something.
>
> Signed-off-by: Juan Quintela
> Reviewed-by: Cédric Le Goater
> ---
> migration/qemu-file.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 4bc875b452..956bd2a580 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -302,7 +302,9 @@ void qemu_fflush(QEMUFile *f)
> &local_error) < 0) {
> qemu_file_set_error_obj(f, -EIO, local_error);
> } else {
> -f->total_transferred += iov_size(f->iov, f->iovcnt);
> +uint64_t size = iov_size(f->iov, f->iovcnt);
> +qemu_file_acct_rate_limit(f, size);
> +f->total_transferred += size;
> }
>
> qemu_iovec_release_ram(f);
> @@ -519,7 +521,6 @@ void qemu_put_buffer_async(QEMUFile *f, const uint8_t
> *buf, size_t size,
> return;
> }
>
> -f->rate_limit_used += size;
> add_to_iovec(f, buf, size, may_free);
> }
>
> @@ -537,7 +538,6 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf,
> size_t size)
> l = size;
> }
> memcpy(f->buf + f->buf_index, buf, l);
> -f->rate_limit_used += l;
> add_buf_to_iovec(f, l);
> if (qemu_file_get_error(f)) {
> break;
> @@ -554,7 +554,6 @@ void qemu_put_byte(QEMUFile *f, int v)
> }
>
> f->buf[f->buf_index] = v;
> -f->rate_limit_used++;
> add_buf_to_iovec(f, 1);
> }
>
If we are counting transferred data at fflush, it makes sense to increase rate-
limit accounting at the same place. It may be less granular, but is more
efficient.
FWIW:
Reviewed-by: Leonardo Bras
Re: [PATCH v3 1/1] hw/arm/aspeed:Add vpd data for Rainier machine
Hello Cedric, On 5/24/23 1:36 AM, Cédric Le Goater wrote: On 5/23/23 23:45, Ninad Palsule wrote: The current modeling of Rainier machine creates zero filled VPDs(EEPROMs). This makes some services and applications unhappy and causing them to fail. Hence this drop adds some fabricated data for system and BMC FRU so that vpd services are happy and active. Tested: - The system-vpd.service is active. - VPD service related to bmc is active. Signed-off-by: Ninad Palsule You can keep the R-b tag when you resend, unless there are a lot of changes. Yes, Sure. Sorry about that. Reviewed-by: Cédric Le Goater Since I am curious, I started a rainier machine under QEMU and ran some commands : root@p10bmc:~# hexdump -C /sys/bus/i2c/devices/i2c-8/8-0050/eeprom 00 00 00 00 00 00 00 00 00 00 00 84 28 00 52 54 |(.RT| 0010 04 56 48 44 52 56 44 02 01 00 50 54 0e 56 54 4f |.VHDRVD...PT.VTO| 0020 43 00 00 37 00 4a 00 00 00 00 00 50 46 08 00 00 |C..7.J.PF...| 0030 00 00 00 00 00 00 00 00 46 00 52 54 04 56 54 4f |F.RT.VTO| 0040 43 50 54 38 56 49 4e 49 00 00 81 00 3a 00 00 00 |CPT8VINI:...| 0050 00 00 56 53 59 53 00 00 bb 00 27 00 00 00 00 00 |..VSYS'.| 0060 56 43 45 4e 00 00 e2 00 27 00 00 00 00 00 56 53 |VCEN'.VS| 0070 42 50 00 00 09 01 19 00 00 00 00 00 50 46 01 00 |BP..PF..| 0080 00 00 36 00 52 54 04 56 49 4e 49 44 52 04 44 45 |..6.RT.VINIDR.DE| 0090 53 43 48 57 02 30 31 43 43 04 33 34 35 36 46 4e |SCHW.01CC.3456FN| 00a0 04 46 52 34 39 53 4e 04 53 52 31 32 50 4e 04 50 |.FR49SN.SR12PN.P| 00b0 52 39 39 50 46 04 00 00 00 00 00 00 23 00 52 54 |R99PF...#.RT| 00c0 04 56 53 59 53 53 45 07 49 42 4d 53 59 53 31 54 |.VSYSSE.IBMSYS1T| 00d0 4d 08 32 32 32 32 2d 32 32 32 50 46 04 00 00 00 |M.-222PF| 00e0 00 00 00 23 00 52 54 04 56 43 45 4e 53 45 07 31 |...#.RT.VCENSE.1| 00f0 32 33 34 35 36 37 46 43 08 31 31 31 31 2d 31 31 |234567FC.-11| 0100 31 50 46 04 00 00 00 00 00 00 15 00 52 54 04 56 |1PF.RT.V| 0110 53 42 50 49 4d 04 50 00 10 01 50 46 04 00 00 00 |SBPIM.P...PF| 0120 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || * 2000 root@p10bmc:~# hexdump -C /sys/bus/i2c/devices/i2c-8/8-0051/eeprom 00 00 00 00 00 00 00 00 00 00 00 84 28 00 52 54 |(.RT| 0010 04 56 48 44 52 56 44 02 01 00 50 54 0e 56 54 4f |.VHDRVD...PT.VTO| 0020 43 00 00 37 00 20 00 00 00 00 00 50 46 08 00 00 |C..7. .PF...| 0030 00 00 00 00 00 00 00 00 1c 00 52 54 04 56 54 4f |..RT.VTO| 0040 43 50 54 0e 56 49 4e 49 00 00 57 00 1e 00 00 00 |CPT.VINI..W.| 0050 00 00 50 46 01 00 00 00 1a 00 52 54 04 56 49 4e |..PF..RT.VIN| 0060 49 44 52 04 44 45 53 43 48 57 02 30 31 50 46 04 |IDR.DESCHW.01PF.| 0070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || and root@p10bmc:~# systemctl status com.ibm.VPD.Manager.service -l * com.ibm.VPD.Manager.service - IBM VPD Manager Loaded: loaded (/lib/systemd/system/com.ibm.VPD.Manager.service; enabled; preset: enabled) Active: active (running) since Wed 2023-05-24 06:26:34 UTC; 1min 28s ago Main PID: 2784 (vpd-manager) CPU: 101ms CGroup: /system.slice/com.ibm.VPD.Manager.service `-2784 /usr/bin/vpd-manager Thank you for checking. But, I also got this : root@p10bmc:~# [ 91.656331] watchdog: watchdog0: watchdog did not stop! [ 91.734858] systemd-shutdown[1]: Using hardware watchdog 'aspeed_wdt', version 0, device /dev/watchdog0 [ 91.735766] systemd-shutdown[1]: Watchdog running with a timeout of 1min. [ 91.987363] systemd-shutdown[1]: Syncing filesystems and block devices. [ 93.471897] systemd-shutdown[1]: Sending SIGTERM to remaining processes... This is a different issue. I am also hitting it without my changes. and the machine rebooted. Joel had the same problem : https://github.com/openbmc/qemu/issues/39 Is it unrelated ? I haven't started a rainier in 2 years at least so I can not tell. Thanks, C. --- hw/arm/aspeed.c | 6 -- hw/arm/aspeed_eeprom.c | 45 +- hw/arm/aspeed_eeprom.h | 5 + 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 0b29028fe1..bfc2070bd2 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -788,8 +788,10 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc) 0x48); i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), TYPE_TMP105, 0x4a); - at24c_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 8), 0x50, 64 * KiB); - at24c_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 8), 0x51, 64 * KiB); + at24c_eeprom_init_rom
Re: [PATCH v2 03/16] migration: Move setup_time to mig_stats
On Mon, 2023-05-15 at 21:56 +0200, Juan Quintela wrote:
> It is a time that needs to be cleaned each time cancel migration.
> Once there create migration_time_since() to calculate how time since a
> time in the past.
>
> Signed-off-by: Juan Quintela
>
> ---
>
> Rename to migration_time_since (cédric)
> ---
> migration/migration-stats.h | 13 +
> migration/migration.h | 1 -
> migration/migration-stats.c | 7 +++
> migration/migration.c | 9 -
> 4 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/migration/migration-stats.h b/migration/migration-stats.h
> index e782f1b0df..21402af9e4 100644
> --- a/migration/migration-stats.h
> +++ b/migration/migration-stats.h
> @@ -75,6 +75,10 @@ typedef struct {
> * Number of bytes sent during precopy stage.
> */
> Stat64 precopy_bytes;
> +/*
> + * How long has the setup stage took.
> + */
> +Stat64 setup_time;
> /*
> * Total number of bytes transferred.
> */
> @@ -87,4 +91,13 @@ typedef struct {
>
> extern MigrationAtomicStats mig_stats;
>
> +/**
> + * migration_time_since: Calculate how much time has passed
> + *
> + * @stats: migration stats
> + * @since: reference time since we want to calculate
> + *
> + * Returns: Nothing. The time is stored in val.
> + */
> +void migration_time_since(MigrationAtomicStats *stats, int64_t since);
> #endif
> diff --git a/migration/migration.h b/migration/migration.h
> index 48a46123a0..27aa3b1035 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -316,7 +316,6 @@ struct MigrationState {
> int64_t downtime;
> int64_t expected_downtime;
> bool capabilities[MIGRATION_CAPABILITY__MAX];
> -int64_t setup_time;
> /*
> * Whether guest was running when we enter the completion stage.
> * If migration is interrupted by any reason, we need to continue
> diff --git a/migration/migration-stats.c b/migration/migration-stats.c
> index 2f2cea965c..3431453c90 100644
> --- a/migration/migration-stats.c
> +++ b/migration/migration-stats.c
> @@ -12,6 +12,13 @@
>
> #include "qemu/osdep.h"
> #include "qemu/stats64.h"
> +#include "qemu/timer.h"
> #include "migration-stats.h"
>
> MigrationAtomicStats mig_stats;
> +
> +void migration_time_since(MigrationAtomicStats *stats, int64_t since)
> +{
> +int64_t now = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> +stat64_set(&stats->setup_time, now - since);
> +}
IIUC this calculates a time delta and saves on stats->setup_time, is that right?
It took me some time to understand that, since the function name is
migration_time_since(), which seems more generic.
Would not be more intuitive to name it migration_setup_time_set() or so?
Anyway,
Reviewed-by: Leonardo Bras
> diff --git a/migration/migration.c b/migration/migration.c
> index c41c7491bb..e9466273bb 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -887,7 +887,7 @@ static void populate_time_info(MigrationInfo *info,
> MigrationState *s)
> {
> info->has_status = true;
> info->has_setup_time = true;
> -info->setup_time = s->setup_time;
> +info->setup_time = stat64_get(&mig_stats.setup_time);
>
> if (s->state == MIGRATION_STATUS_COMPLETED) {
> info->has_total_time = true;
> @@ -1390,7 +1390,6 @@ void migrate_init(MigrationState *s)
> s->pages_per_second = 0.0;
> s->downtime = 0;
> s->expected_downtime = 0;
> -s->setup_time = 0;
I could not see MigrationState->setup_time being initialized as 0 in this patch.
In a quick look in the code I noticed there is no initialization of this struct,
but on qemu_savevm_state() and migrate_prepare() we have:
memset(&mig_stats, 0, sizeof(mig_stats));
I suppose this is enough, right?
> s->start_postcopy = false;
> s->postcopy_after_devices = false;
> s->migration_thread_running = false;
> @@ -2647,7 +2646,7 @@ static void migration_calculate_complete(MigrationState
> *s)
> s->downtime = end_time - s->downtime_start;
> }
>
> -transfer_time = s->total_time - s->setup_time;
> +transfer_time = s->total_time - stat64_get(&mig_stats.setup_time);
> if (transfer_time) {
> s->mbps = ((double) bytes * 8.0) / transfer_time / 1000;
> }
> @@ -2969,7 +2968,7 @@ static void *migration_thread(void *opaque)
> qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP,
> MIGRATION_STATUS_ACTIVE);
>
> -s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
> +migration_time_since(&mig_stats, setup_start);
>
> trace_migration_thread_setup_complete();
>
> @@ -3081,7 +3080,7 @@ static void *bg_migration_thread(void *opaque)
> qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP,
> MIGRATION_STATUS_ACTIVE);
>
> -s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
> +migration_time_since(&mig_stats, setup_start);
>
Re: [RFC PATCH v1 0/9] Hypervisor-Enforced Kernel Integrity
On Fri, 2023-05-05 at 17:20 +0200, Mickaël Salaün wrote: > # How does it work? > > This implementation mainly leverages KVM capabilities to control the > Second > Layer Address Translation (or the Two Dimensional Paging e.g., > Intel's EPT or > AMD's RVI/NPT) and Mode Based Execution Control (Intel's MBEC) > introduced with > the Kaby Lake (7th generation) architecture. This allows to set > permissions on > memory pages in a complementary way to the guest kernel's managed > memory > permissions. Once these permissions are set, they are locked and > there is no > way back. > > A first KVM_HC_LOCK_MEM_PAGE_RANGES hypercall enables the guest > kernel to lock > a set of its memory page ranges with either the HEKI_ATTR_MEM_NOWRITE > or the > HEKI_ATTR_MEM_EXEC attribute. The first one denies write access to a > specific > set of pages (allow-list approach), and the second only allows kernel > execution > for a set of pages (deny-list approach). > > The current implementation sets the whole kernel's .rodata (i.e., any > const or > __ro_after_init variables, which includes critical security data such > as LSM > parameters) and .text sections as non-writable, and the .text section > is the > only one where kernel execution is allowed. This is possible thanks > to the new > MBEC support also brough by this series (otherwise the vDSO would > have to be > executable). Thanks to this hardware support (VT-x, EPT and MBEC), > the > performance impact of such guest protection is negligible. > > The second KVM_HC_LOCK_CR_UPDATE hypercall enables guests to pin some > of its > CPU control register flags (e.g., X86_CR0_WP, X86_CR4_SMEP, > X86_CR4_SMAP), > which is another complementary hardening mechanism. > > Heki can be enabled with the heki=1 boot command argument. > > Can the guest kernel ask the host VMM's emulated devices to DMA into the protected data? It should go through the host userspace mappings I think, which don't care about EPT permissions. Or did I miss where you are protecting that another way? There are a lot of easy ways to ask the host to write to guest memory that don't involve the EPT. You probably need to protect the host userspace mappings, and also the places in KVM that kmap a GPA provided by the guest. [ snip ] > > # Current limitations > > The main limitation of this patch series is the statically enforced > permissions. This is not an issue for kernels without module but this > needs to > be addressed. Mechanisms that dynamically impact kernel executable > memory are > not handled for now (e.g., kernel modules, tracepoints, eBPF JIT), > and such > code will need to be authenticated. Because the hypervisor is highly > privileged and critical to the security of all the VMs, we don't want > to > implement a code authentication mechanism in the hypervisor itself > but delegate > this verification to something much less privileged. We are thinking > of two > ways to solve this: implement this verification in the VMM or spawn a > dedicated > special VM (similar to Windows's VBS). There are pros on cons to each > approach: > complexity, verification code ownership (guest's or VMM's), access to > guest > memory (i.e., confidential computing). The kernel often creates writable aliases in order to write to protected data (kernel text, etc). Some of this is done right as text is being first written out (alternatives for example), and some happens way later (jump labels, etc). So for verification, I wonder what stage you would be verifying? If you want to verify the end state, you would have to maintain knowledge in the verifier of all the touch-ups the kernel does. I think it would get very tricky. It also seems it will be a decent ask for the guest kernel to keep track of GPA permissions as well as normal virtual memory pemirssions, if this thing is not widely used. So I wondering if you could go in two directions with this: 1. Make this a feature only for super locked down kernels (no modules, etc). Forbid any configurations that might modify text. But eBPF is used for seccomp, so you might be turning off some security protections to get this. 2. Loosen the rules to allow the protections to not be so one-way enable. Get less security, but used more widely. There were similar dilemmas with the PV CR pinning stuff.
Re: [PULL 0/2] vfio queue
On 5/24/23 01:47, Cédric Le Goater wrote: The following changes since commit aa33508196f4e2da04625bee36e1f7be5b9267e7: Merge tag 'mem-2023-05-23' ofhttps://github.com/davidhildenbrand/qemu into staging (2023-05-23 10:57:25 -0700) are available in the Git repository at: https://github.com/legoater/qemu/ tags/pull-vfio-20230524 for you to fetch changes up to dbdea0dbfe2cef9ef6c752e9077e4fc98724194c: util/vfio-helpers: Use g_file_read_link() (2023-05-24 09:21:22 +0200) vfio queue: * Fix for a memory corruption due to an extra free * Fix for a compile breakage Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate. r~
Re: [PATCH v12 02/15] accel: collecting TB execution count
On 5/25/2023 1:02 AM, Richard Henderson wrote:
> On 5/24/23 06:35, Wu, Fei wrote:
>> On 5/23/2023 8:45 AM, Richard Henderson wrote:
>>> On 5/18/23 06:57, Fei Wu wrote:
+void HELPER(inc_exec_freq)(void *ptr)
+{
+ TBStatistics *stats = (TBStatistics *) ptr;
+ tcg_debug_assert(stats);
+ ++stats->executions.normal;
+}
>>> ...
+static inline void gen_tb_exec_count(TranslationBlock *tb)
+{
+ if (tb_stats_enabled(tb, TB_EXEC_STATS)) {
+ TCGv_ptr ptr = tcg_temp_new_ptr();
+ tcg_gen_movi_ptr(ptr, (intptr_t)tb->tb_stats);
+ gen_helper_inc_exec_freq(ptr);
+ }
+}
>>>
>>> This is 3 host instructions, easily expanded inline:
>>>
>>> --- a/accel/tcg/translator.c
>>> +++ b/accel/tcg/translator.c
>>> @@ -11,6 +11,7 @@
>>> #include "qemu/error-report.h"
>>> #include "tcg/tcg.h"
>>> #include "tcg/tcg-op.h"
>>> +#include "tcg/tcg-temp-internal.h"
>>> #include "exec/exec-all.h"
>>> #include "exec/gen-icount.h"
>>> #include "exec/log.h"
>>> @@ -18,6 +19,30 @@
>>> #include "exec/plugin-gen.h"
>>> #include "exec/replay-core.h"
>>>
>>> +
>>> +static void gen_tb_exec_count(TranslationBlock *tb)
>>> +{
>>> + if (tb_stats_enabled(tb, TB_EXEC_STATS)) {
>>> + TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
>>> +
>>> + tcg_gen_movi_ptr(ptr,
>>> (intptr_t)&tb->tb_stats->executions.normal);
>>> + if (sizeof(tb->tb_stats->executions.normal) == 4) {
>>> + TCGv_i32 t = tcg_temp_ebb_new_i32();
>>> + tcg_gen_ld_i32(t, ptr, 0);
>>> + tcg_gen_addi_i32(t, t, 1);
>>> + tcg_gen_st_i32(t, ptr, 0);
>>> + tcg_temp_free_i32(t);
>>> + } else {
>>> + TCGv_i64 t = tcg_temp_ebb_new_i64();
>>> + tcg_gen_ld_i64(t, ptr, 0);
>>> + tcg_gen_addi_i64(t, t, 1);
>>> + tcg_gen_st_i64(t, ptr, 0);
>>> + tcg_temp_free_i64(t);
>>> + }
>>> + tcg_temp_free_ptr(ptr);
>>> + }
>>> +}
>>> +
>>> bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
>>> {
>>> /* Suppress goto_tb if requested. */
>>>
>>>
>>> I'm not expecially keen on embedding the TBStatistics pointer directly
>>> like this; for most hosts we will have to put this constant into the
>>> constant pool. Whereas the pointer already exists at tb->tb_stats, and
>>> tb is at a constant displacement prior to the code, so we already have
>>> mechanisms for generating pc-relative addresses.
>>>
>>> However, that's premature optimization. Let's get it working first.
>>>
>> Richard, have you reviewed the whole series? I will integrate your
>> change to next version.
>
> No, it's difficult to see what's going on.
> In your next revision, please remove CONFIG_PROFILER entirely first,
> which was what I was planning to do locally.
>
OK, I will update it.
Thanks,
Fei.
>
> r~
Re: [RFC PATCH v1 0/9] Hypervisor-Enforced Kernel Integrity
On 5/24/2023 3:20 PM, Edgecombe, Rick P wrote: On Fri, 2023-05-05 at 17:20 +0200, Mickaël Salaün wrote: # How does it work? This implementation mainly leverages KVM capabilities to control the Second Layer Address Translation (or the Two Dimensional Paging e.g., Intel's EPT or AMD's RVI/NPT) and Mode Based Execution Control (Intel's MBEC) introduced with the Kaby Lake (7th generation) architecture. This allows to set permissions on memory pages in a complementary way to the guest kernel's managed memory permissions. Once these permissions are set, they are locked and there is no way back. A first KVM_HC_LOCK_MEM_PAGE_RANGES hypercall enables the guest kernel to lock a set of its memory page ranges with either the HEKI_ATTR_MEM_NOWRITE or the HEKI_ATTR_MEM_EXEC attribute. The first one denies write access to a specific set of pages (allow-list approach), and the second only allows kernel execution for a set of pages (deny-list approach). The current implementation sets the whole kernel's .rodata (i.e., any const or __ro_after_init variables, which includes critical security data such as LSM parameters) and .text sections as non-writable, and the .text section is the only one where kernel execution is allowed. This is possible thanks to the new MBEC support also brough by this series (otherwise the vDSO would have to be executable). Thanks to this hardware support (VT-x, EPT and MBEC), the performance impact of such guest protection is negligible. The second KVM_HC_LOCK_CR_UPDATE hypercall enables guests to pin some of its CPU control register flags (e.g., X86_CR0_WP, X86_CR4_SMEP, X86_CR4_SMAP), which is another complementary hardening mechanism. Heki can be enabled with the heki=1 boot command argument. Can the guest kernel ask the host VMM's emulated devices to DMA into the protected data? It should go through the host userspace mappings I think, which don't care about EPT permissions. Or did I miss where you are protecting that another way? There are a lot of easy ways to ask the host to write to guest memory that don't involve the EPT. You probably need to protect the host userspace mappings, and also the places in KVM that kmap a GPA provided by the guest. [ snip ] # Current limitations The main limitation of this patch series is the statically enforced permissions. This is not an issue for kernels without module but this needs to be addressed. Mechanisms that dynamically impact kernel executable memory are not handled for now (e.g., kernel modules, tracepoints, eBPF JIT), and such code will need to be authenticated. Because the hypervisor is highly privileged and critical to the security of all the VMs, we don't want to implement a code authentication mechanism in the hypervisor itself but delegate this verification to something much less privileged. We are thinking of two ways to solve this: implement this verification in the VMM or spawn a dedicated special VM (similar to Windows's VBS). There are pros on cons to each approach: complexity, verification code ownership (guest's or VMM's), access to guest memory (i.e., confidential computing). The kernel often creates writable aliases in order to write to protected data (kernel text, etc). Some of this is done right as text is being first written out (alternatives for example), and some happens way later (jump labels, etc). So for verification, I wonder what stage you would be verifying? If you want to verify the end state, you would have to maintain knowledge in the verifier of all the touch-ups the kernel does. I think it would get very tricky. Right and for the ARM (from what I know) is that Erratas can be applied using the alternatives fwk when you hotplug in the CPU post boot. ---Trilok Soni
Re: [PATCH v3 1/1] hw/arm/aspeed:Add vpd data for Rainier machine
On Wed, 24 May 2023, at 17:14, Joel Stanley wrote: > On Wed, 24 May 2023 at 06:38, Cédric Le Goater wrote: >> >> But, I also got this : >> >>root@p10bmc:~# [ 91.656331] watchdog: watchdog0: watchdog did not stop! >>[ 91.734858] systemd-shutdown[1]: Using hardware watchdog 'aspeed_wdt', >> version 0, device /dev/watchdog0 >>[ 91.735766] systemd-shutdown[1]: Watchdog running with a timeout of >> 1min. >>[ 91.987363] systemd-shutdown[1]: Syncing filesystems and block devices. >>[ 93.471897] systemd-shutdown[1]: Sending SIGTERM to remaining >> processes... >> >> and the machine rebooted. >> >> Joel had the same problem : >> >>https://github.com/openbmc/qemu/issues/39 >> >> Is it unrelated ? I haven't started a rainier in 2 years at least so I can >> not tell. > > I don't think it's related to Ninad's patches. > > I am able to reproduce the issue on my old Skylake x86 machine, but it > doesn't happen on my M1 mac mini. > > I suspect the emulation is moving too slowly, but the host's wall > clock is still ticking, so all of a sudden the BMC finds out that time > has passed an the watchdog bites. I could be wrong. Yeah, I also suspect this is occurring. In some cases we see output like: ``` [ 295.474921] hrtimer: interrupt took 411679950 ns Watchdog timer 0 expired. ``` This suggests that we've exhausted the three iterations of the hrtimer callback list from the hrtimer interrupt[1]. Too much stuff happening for the time-keeping we have. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/time/hrtimer.c?h=v6.3#n1869 However, regarding Cédric's log above, a reboot is expected on the first boot of a fresh image when there's valid VPD available. For the first boot of a fresh image we configure the kernel with a minimal devicetree that allows us to read the VPD data. This determines the system we're actually on and configures an appropriate devicetree for subsequent boots. We then reboot to pick up the new devicetree. The devicetree to use is stored in the u-boot environment (in `fitconfig`). Andrew
Re: [PATCH v7 08/14] KVM: Rename mmu_notifier_*
On Wed, May 24, 2023, Peter Zijlstra wrote:
> On Wed, May 24, 2023 at 01:16:03PM -0700, Sean Christopherson wrote:
> > Of course, the only accesses outside of mmu_lock are reads, so on x86 that
> > "atomic" access is just a READ_ONCE() load, but that's not the case for all
> > architectures.
>
> This is true on *all* archs. atomic_set() and atomic_read() are no more
> and no less than WRITE_ONCE() / READ_ONCE().
Ah, I take it s390's handcoded assembly routines are just a paranoid equivalents
and not truly special? "l" and "st" do sound quite generic...
commit 7657e41a0bd16c9d8b3cefe8fd5d6ac3c25ae4bf
Author: Heiko Carstens
Date: Thu Feb 17 13:13:58 2011 +0100
[S390] atomic: use inline asm
Use inline assemblies for atomic_read/set(). This way there shouldn't
be any questions or subtle volatile semantics left.
static inline int __atomic_read(const atomic_t *v)
{
int c;
asm volatile(
" l %0,%1\n"
: "=d" (c) : "R" (v->counter));
return c;
}
static inline void __atomic_set(atomic_t *v, int i)
{
asm volatile(
" st %1,%0\n"
: "=R" (v->counter) : "d" (i));
}
[PATCH] target/i386: EPYC-Rome model without XSAVES
Based on the kernel commit "b0563468ee x86/CPU/AMD: Disable XSAVES on
AMD family 0x17", host system with EPYC-Rome can clear XSAVES capability
bit. In another words, EPYC-Rome host without XSAVES can occur. Thus, we
need an EPYC-Rome cpu model (without this feature) that matches the
solution of fixing this erratum
Signed-off-by: Maksim Davydov
---
target/i386/cpu.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a61cd6d99d1f..1242bd541a53 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4466,6 +4466,16 @@ static const X86CPUDefinition builtin_x86_defs[] = {
},
.cache_info = &epyc_rome_v3_cache_info
},
+{
+.version = 4,
+.props = (PropValue[]) {
+/* Erratum 1386 */
+{ "model-id",
+ "AMD EPYC-Rome-v4 Processor (no XSAVES)" },
+{ "xsaves", "off" },
+{ /* end of list */ }
+},
+},
{ /* end of list */ }
}
},
--
2.25.1
Re: Add CI configuration for Kubernetes
On 5/22/23 10:41, Camilla Conte wrote: Here's a second version (v2) of patches to support the Kubernetes runner for Gitlab CI. You can find the v1 thread here: https://lore.kernel.org/qemu-devel/20230407145252.32955-1-cco...@redhat.com/. Applied to master. With the k8s tag on the azure runner, it seems to work. r~
[PATCH 25/30] mac_via: implement ADB_STATE_IDLE state if shift register in input mode
NetBSD switches directly to IDLE state without switching the shift register to
input mode. Duplicate the existing ADB_STATE_IDLE logic in input mode from when
the shift register is in output mode which allows the ADB autopoll handler to
handle the response.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 564db8337e..c1d2866ec9 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -702,6 +702,12 @@ static void adb_via_send(MOS6522Q800VIA1State *v1s, int
state, uint8_t data)
break;
case ADB_STATE_IDLE:
+ms->b |= VIA1B_vADBInt;
+adb_autopoll_unblock(adb_bus);
+
+trace_via1_adb_send("IDLE", data,
+(ms->b & VIA1B_vADBInt) ? "+" : "-");
+
return;
}
--
2.30.2
[PATCH 16/30] q800: add Apple Sound Chip (ASC) audio to machine
The Quadra 800 has the enhanced ASC (EASC) audio chip which supports both the
legacy IRQ routing through VIA2 and also "A/UX" mode routing direct to the
CPU.
Co-developed-by: Laurent Vivier
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index d12aeaa20e..ed862f9e9d 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -43,6 +43,7 @@
#include "hw/misc/djmemc.h"
#include "hw/misc/iosb.h"
#include "hw/input/adb.h"
+#include "hw/audio/asc.h"
#include "hw/nubus/mac-nubus-bridge.h"
#include "hw/display/macfb.h"
#include "hw/block/swim.h"
@@ -115,7 +116,7 @@ struct GLUEState {
M68kCPU *cpu;
uint8_t ipr;
uint8_t auxmode;
-qemu_irq irqs[1];
+qemu_irq irqs[2];
QEMUTimer *nmi_release;
};
@@ -124,8 +125,10 @@ struct GLUEState {
#define GLUE_IRQ_IN_SONIC 2
#define GLUE_IRQ_IN_ESCC 3
#define GLUE_IRQ_IN_NMI4
+#define GLUE_IRQ_IN_ASC5
#define GLUE_IRQ_NUBUS_9 0
+#define GLUE_IRQ_ASC 1
/*
* The GLUE logic on the Quadra 800 supports 2 different IRQ routing modes
@@ -187,6 +190,11 @@ static void GLUE_set_irq(void *opaque, int irq, int level)
irq = 6;
break;
+case GLUE_IRQ_IN_ASC:
+/* Route to VIA2 instead, negative edge-triggered */
+qemu_set_irq(s->irqs[GLUE_IRQ_ASC], !level);
+return;
+
default:
g_assert_not_reached();
}
@@ -213,6 +221,10 @@ static void GLUE_set_irq(void *opaque, int irq, int level)
irq = 6;
break;
+case GLUE_IRQ_IN_ASC:
+irq = 4;
+break;
+
default:
g_assert_not_reached();
}
@@ -304,7 +316,7 @@ static void glue_init(Object *obj)
qdev_init_gpio_in(dev, GLUE_set_irq, 8);
qdev_init_gpio_in_named(dev, glue_auxmode_set_irq, "auxmode", 1);
-qdev_init_gpio_out(dev, s->irqs, 1);
+qdev_init_gpio_out(dev, s->irqs, 2);
/* NMI release timer */
s->nmi_release = timer_new_ms(QEMU_CLOCK_VIRTUAL, glue_nmi_release, s);
@@ -695,6 +707,20 @@ static void q800_machine_init(MachineState *machine)
scsi_bus_legacy_handle_cmdline(&esp->bus);
+/* Apple Sound Chip */
+
+dev = qdev_new(TYPE_ASC);
+qdev_prop_set_uint8(dev, "asctype", ASC_TYPE_EASC);
+sysbus = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(sysbus, &error_fatal);
+memory_region_add_subregion(&m->macio, ASC_BASE - IO_BASE,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(dev),
0));
+sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(m->glue, GLUE_IRQ_IN_ASC));
+
+/* Wire ASC IRQ via GLUE for use in classic mode */
+qdev_connect_gpio_out(m->glue, GLUE_IRQ_ASC,
+ qdev_get_gpio_in(via2_dev, VIA2_IRQ_ASC_BIT));
+
/* SWIM floppy controller */
dev = qdev_new(TYPE_SWIM);
--
2.30.2
[PATCH 19/30] swim: split into separate IWM and ISM register blocks
The swim chip provides an implementation of both Apple's IWM and ISM floppy disk
controllers. Split the existing implementation into separate register banks for
each controller, whilst also switching the IWM registers from 16-bit to 8-bit
as implemented in real hardware.
Signed-off-by: Mark Cave-Ayland
---
hw/block/swim.c | 85 -
hw/block/trace-events | 4 +-
include/hw/block/swim.h | 15 +++-
3 files changed, 58 insertions(+), 46 deletions(-)
diff --git a/hw/block/swim.c b/hw/block/swim.c
index 7df36ea139..735b335883 100644
--- a/hw/block/swim.c
+++ b/hw/block/swim.c
@@ -126,7 +126,14 @@
#define SWIM_HEDSEL 0x20
#define SWIM_MOTON 0x80
-static const char *swim_reg_names[] = {
+static const char *iwm_reg_names[] = {
+"PH0L", "PH0H", "PH1L", "PH1H",
+"PH2L", "PH2H", "PH3L", "PH3H",
+"MTROFF", "MTRON", "INTDRIVE", "EXTDRIVE",
+"Q6L", "Q6H", "Q7L", "Q7H"
+};
+
+static const char *ism_reg_names[] = {
"WRITE_DATA", "WRITE_MARK", "WRITE_CRC", "WRITE_PARAMETER",
"WRITE_PHASE", "WRITE_SETUP", "WRITE_MODE0", "WRITE_MODE1",
"READ_DATA", "READ_MARK", "READ_ERROR", "READ_PARAMETER",
@@ -274,12 +281,11 @@ static void iwmctrl_write(void *opaque, hwaddr reg,
uint64_t value,
reg >>= REG_SHIFT;
-swimctrl->regs[reg >> 1] = reg & 1;
-trace_swim_iwmctrl_write((reg >> 1), size, (reg & 1));
+swimctrl->iwmregs[reg] = value;
+trace_swim_iwmctrl_write(reg, iwm_reg_names[reg], size, value);
-if (swimctrl->regs[IWM_Q6] &&
-swimctrl->regs[IWM_Q7]) {
-if (swimctrl->regs[IWM_MTR]) {
+if (swimctrl->iwmregs[IWM_Q7H]) {
+if (swimctrl->iwmregs[IWM_MTRON]) {
/* data register */
swimctrl->iwm_data = value;
} else {
@@ -307,6 +313,12 @@ static void iwmctrl_write(void *opaque, hwaddr reg,
uint64_t value,
swimctrl->mode = SWIM_MODE_SWIM;
swimctrl->iwm_switch = 0;
trace_swim_iwm_switch();
+
+/* Switch to ISM registers */
+memory_region_del_subregion(&swimctrl->swim,
+&swimctrl->iwm);
+memory_region_add_subregion(&swimctrl->swim, 0x0,
+&swimctrl->ism);
}
break;
}
@@ -317,28 +329,30 @@ static void iwmctrl_write(void *opaque, hwaddr reg,
uint64_t value,
static uint64_t iwmctrl_read(void *opaque, hwaddr reg, unsigned size)
{
SWIMCtrl *swimctrl = opaque;
+uint16_t value;
reg >>= REG_SHIFT;
-swimctrl->regs[reg >> 1] = reg & 1;
+value = swimctrl->iwmregs[reg];
+trace_swim_iwmctrl_read(reg, iwm_reg_names[reg], size, value);
-trace_swim_iwmctrl_read((reg >> 1), size, (reg & 1));
-return 0;
+return value;
}
-static void swimctrl_write(void *opaque, hwaddr reg, uint64_t value,
- unsigned size)
+static const MemoryRegionOps swimctrl_iwm_ops = {
+.write = iwmctrl_write,
+.read = iwmctrl_read,
+.endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void ismctrl_write(void *opaque, hwaddr reg, uint64_t value,
+ unsigned size)
{
SWIMCtrl *swimctrl = opaque;
-if (swimctrl->mode == SWIM_MODE_IWM) {
-iwmctrl_write(opaque, reg, value, size);
-return;
-}
-
reg >>= REG_SHIFT;
-trace_swim_swimctrl_write(reg, swim_reg_names[reg], size, value);
+trace_swim_swimctrl_write(reg, ism_reg_names[reg], size, value);
switch (reg) {
case SWIM_WRITE_PHASE:
@@ -359,15 +373,11 @@ static void swimctrl_write(void *opaque, hwaddr reg,
uint64_t value,
}
}
-static uint64_t swimctrl_read(void *opaque, hwaddr reg, unsigned size)
+static uint64_t ismctrl_read(void *opaque, hwaddr reg, unsigned size)
{
SWIMCtrl *swimctrl = opaque;
uint32_t value = 0;
-if (swimctrl->mode == SWIM_MODE_IWM) {
-return iwmctrl_read(opaque, reg, size);
-}
-
reg >>= REG_SHIFT;
switch (reg) {
@@ -389,14 +399,14 @@ static uint64_t swimctrl_read(void *opaque, hwaddr reg,
unsigned size)
break;
}
-trace_swim_swimctrl_read(reg, swim_reg_names[reg], size, value);
+trace_swim_swimctrl_read(reg, ism_reg_names[reg], size, value);
return value;
}
-static const MemoryRegionOps swimctrl_mem_ops = {
-.write = swimctrl_write,
-.read = swimctrl_read,
-.endianness = DEVICE_NATIVE_ENDIAN,
+static const MemoryRegionOps swimctrl_ism_ops = {
+.write = ismctrl_write,
+.read = ismctrl_read,
+.endianness = DEVICE_BIG_ENDIAN,
};
static void sysbus_swim_reset(DeviceState *d)
@@ -407,13 +417,13 @@ static void sysbus_swim_reset(DeviceState *d)
ctrl->mode = 0;
ctrl->iwm_switch = 0;
-for (i = 0; i < 8; i++) {
-ctrl->regs[i] = 0;
-}
ctrl->iwm_data = 0;
ctrl->iwm_mode = 0;
[PATCH 12/30] q800: add IOSB subsystem
It is needed because it defines the BIOSConfig area.
Co-developed-by: Laurent Vivier
Signed-off-by: Mark Cave-Ayland
---
MAINTAINERS| 2 +
hw/m68k/Kconfig| 1 +
hw/m68k/q800.c | 10 +++
hw/misc/Kconfig| 3 +
hw/misc/iosb.c | 156 +
hw/misc/meson.build| 1 +
hw/misc/trace-events | 4 ++
include/hw/misc/iosb.h | 41 +++
8 files changed, 218 insertions(+)
create mode 100644 hw/misc/iosb.c
create mode 100644 include/hw/misc/iosb.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 21ec70d00a..f151aaf99f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1228,6 +1228,7 @@ F: hw/nubus/*
F: hw/display/macfb.c
F: hw/block/swim.c
F: hw/misc/djmemc.c
+F: hw/misc/iosb.c
F: hw/m68k/bootinfo.h
F: include/standard-headers/asm-m68k/bootinfo.h
F: include/standard-headers/asm-m68k/bootinfo-mac.h
@@ -1237,6 +1238,7 @@ F: include/hw/display/macfb.h
F: include/hw/block/swim.h
F: include/hw/m68k/q800.h
F: include/hw/misc/djmemc.c
+F: include/hw/misc/iosb.c
virt
M: Laurent Vivier
diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig
index 330cfdfa2d..64fa70a0db 100644
--- a/hw/m68k/Kconfig
+++ b/hw/m68k/Kconfig
@@ -24,6 +24,7 @@ config Q800
select DP8393X
select OR_IRQ
select DJMEMC
+select IOSB
config M68K_VIRT
bool
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index c1d4b98cc0..8310670ec2 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -41,6 +41,7 @@
#include "hw/m68k/q800.h"
#include "hw/misc/mac_via.h"
#include "hw/misc/djmemc.h"
+#include "hw/misc/iosb.h"
#include "hw/input/adb.h"
#include "hw/nubus/mac-nubus-bridge.h"
#include "hw/display/macfb.h"
@@ -71,6 +72,7 @@
#define ESP_BASE (IO_BASE + 0x1)
#define ESP_PDMA (IO_BASE + 0x10100)
#define ASC_BASE (IO_BASE + 0x14000)
+#define IOSB_BASE (IO_BASE + 0x18000)
#define SWIM_BASE (IO_BASE + 0x1E000)
#define SONIC_PROM_SIZE 0x1000
@@ -530,6 +532,14 @@ static void q800_machine_init(MachineState *machine)
memory_region_add_subregion(&m->macio, DJMEMC_BASE - IO_BASE,
sysbus_mmio_get_region(sysbus, 0));
+/* IOSB subsystem */
+
+dev = qdev_new(TYPE_IOSB);
+sysbus = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(sysbus, &error_fatal);
+memory_region_add_subregion(&m->macio, IOSB_BASE - IO_BASE,
+sysbus_mmio_get_region(sysbus, 0));
+
/* VIA 1 */
via1_dev = qdev_new(TYPE_MOS6522_Q800_VIA1);
dinfo = drive_get(IF_MTD, 0, 0);
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 7eaf955f88..c6c38102b1 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -183,4 +183,7 @@ config AXP209_PMU
config DJMEMC
bool
+config IOSB
+bool
+
source macio/Kconfig
diff --git a/hw/misc/iosb.c b/hw/misc/iosb.c
new file mode 100644
index 00..f4f9b22d89
--- /dev/null
+++ b/hw/misc/iosb.c
@@ -0,0 +1,156 @@
+/*
+ * QEMU IOSB emulation
+ *
+ * Copyright (c) 2019 Laurent Vivier
+ * Copyright (c) 2022 Mark Cave-Ayland
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "migration/vmstate.h"
+#include "hw/sysbus.h"
+#include "hw/misc/iosb.h"
+#include "trace.h"
+
+#define IOSB_SIZE 0x2000
+
+#define IOSB_CONFIG0x0
+#define IOSB_CONFIG2 0x100
+#define IOSB_SONIC_SCSI0x200
+#define IOSB_REVISION 0x300
+#define IOSB_SCSI_RESID0x400
+#define IOSB_BRIGHTNESS0x500
+#define IOSB_TIMEOUT 0x600
+
+
+static uint64_t iosb_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+IOSBState *s = IOSB(opaque);
+uint64_t val = 0;
+
+switch (addr) {
+case IOSB_CONFIG:
+case IOSB_CONFIG2:
+case IOSB_SONIC_SCSI:
+case IOSB_REVISION:
+case IOSB_SCSI_RESID:
+case IOSB_BRIGHTNESS:
+case IOSB_T
[PATCH 15/30] asc: generate silence if FIFO empty but engine still running
MacOS (un)helpfully leaves the FIFO engine running even when all the samples
have
been written to the hardware, and expects the FIFO status flags and IRQ to be
updated continuously.
Since not all audio backends guarantee an all-zero output when no data is
provided, explicitly generate an all-zero output when this condition occurs to
avoid the audio backends re-using their internal buffers and looping audio once
the FIFOs are empty.
Signed-off-by: Mark Cave-Ayland
---
hw/audio/asc.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/audio/asc.c b/hw/audio/asc.c
index 04194b1e43..c5173a8d35 100644
--- a/hw/audio/asc.c
+++ b/hw/audio/asc.c
@@ -158,8 +158,10 @@ static int generate_fifo(ASCState *s, int maxsamples)
limit = MIN(MAX(s->fifos[0].cnt, s->fifos[1].cnt), maxsamples);
/*
- * If starting a new run with no FIFO data present, update the IRQ and
- * continue
+ * MacOS (un)helpfully leaves the FIFO engine running even when it has
+ * finished writing out samples. Since not all audio backends guarantee an
+ * all-zero output when no data is provided, zero out the sample buffer
+ * and then update the FIFO flags and IRQ as normal and continue
*/
if (limit == 0 && s->fifos[0].int_status == 0 &&
s->fifos[1].int_status == 0) {
@@ -168,8 +170,9 @@ static int generate_fifo(ASCState *s, int maxsamples)
s->fifos[1].int_status |= ASC_FIFO_STATUS_HALF_FULL |
ASC_FIFO_STATUS_FULL_EMPTY;
+memset(buf, 0x80, maxsamples << s->shift);
asc_raise_irq(s);
-return 0;
+return maxsamples;
}
while (count < limit) {
--
2.30.2
[PATCH 18/30] swim: add trace events for IWM and ISM registers
Signed-off-by: Mark Cave-Ayland
---
hw/block/swim.c | 14 ++
hw/block/trace-events | 7 +++
2 files changed, 21 insertions(+)
diff --git a/hw/block/swim.c b/hw/block/swim.c
index 333da08ce0..7df36ea139 100644
--- a/hw/block/swim.c
+++ b/hw/block/swim.c
@@ -19,6 +19,7 @@
#include "hw/block/block.h"
#include "hw/block/swim.h"
#include "hw/qdev-properties.h"
+#include "trace.h"
/* IWM registers */
@@ -125,6 +126,13 @@
#define SWIM_HEDSEL 0x20
#define SWIM_MOTON 0x80
+static const char *swim_reg_names[] = {
+"WRITE_DATA", "WRITE_MARK", "WRITE_CRC", "WRITE_PARAMETER",
+"WRITE_PHASE", "WRITE_SETUP", "WRITE_MODE0", "WRITE_MODE1",
+"READ_DATA", "READ_MARK", "READ_ERROR", "READ_PARAMETER",
+"READ_PHASE", "READ_SETUP", "READ_STATUS", "READ_HANDSHAKE"
+};
+
static void fd_recalibrate(FDrive *drive)
{
}
@@ -267,6 +275,7 @@ static void iwmctrl_write(void *opaque, hwaddr reg,
uint64_t value,
reg >>= REG_SHIFT;
swimctrl->regs[reg >> 1] = reg & 1;
+trace_swim_iwmctrl_write((reg >> 1), size, (reg & 1));
if (swimctrl->regs[IWM_Q6] &&
swimctrl->regs[IWM_Q7]) {
@@ -297,6 +306,7 @@ static void iwmctrl_write(void *opaque, hwaddr reg,
uint64_t value,
if (value == 0x57) {
swimctrl->mode = SWIM_MODE_SWIM;
swimctrl->iwm_switch = 0;
+trace_swim_iwm_switch();
}
break;
}
@@ -312,6 +322,7 @@ static uint64_t iwmctrl_read(void *opaque, hwaddr reg,
unsigned size)
swimctrl->regs[reg >> 1] = reg & 1;
+trace_swim_iwmctrl_read((reg >> 1), size, (reg & 1));
return 0;
}
@@ -327,6 +338,8 @@ static void swimctrl_write(void *opaque, hwaddr reg,
uint64_t value,
reg >>= REG_SHIFT;
+trace_swim_swimctrl_write(reg, swim_reg_names[reg], size, value);
+
switch (reg) {
case SWIM_WRITE_PHASE:
swimctrl->swim_phase = value;
@@ -376,6 +389,7 @@ static uint64_t swimctrl_read(void *opaque, hwaddr reg,
unsigned size)
break;
}
+trace_swim_swimctrl_read(reg, swim_reg_names[reg], size, value);
return value;
}
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 34be8b9135..c041ec45e3 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -90,3 +90,10 @@ m25p80_read_data(void *s, uint32_t pos, uint8_t v) "[%p]
Read data 0x%"PRIx32"=0
m25p80_read_sfdp(void *s, uint32_t addr, uint8_t v) "[%p] Read SFDP
0x%"PRIx32"=0x%"PRIx8
m25p80_binding(void *s) "[%p] Binding to IF_MTD drive"
m25p80_binding_no_bdrv(void *s) "[%p] No BDRV - binding to RAM"
+
+# swim.c
+swim_swimctrl_read(int reg, const char *name, unsigned size, uint64_t value)
"reg=%d [%s] size=%u value=0x%"PRIx64
+swim_swimctrl_write(int reg, const char *name, unsigned size, uint64_t value)
"reg=%d [%s] size=%u value=0x%"PRIx64
+swim_iwmctrl_read(int reg, unsigned size, uint64_t value) "reg=%d size=%u
value=0x%"PRIx64
+swim_iwmctrl_write(int reg, unsigned size, uint64_t value) "reg=%d size=%u
value=0x%"PRIx64
+swim_iwm_switch(void) "switch from IWM to SWIM mode"
--
2.30.2
[PATCH 20/30] swim: update IWM/ISM register block decoding
Update the IWM/ISM register block decoding to match the description given in the
"SWIM Chip Users Reference". This allows us to validate the device response to
the guest OS which currently only does just enough to indicate that the floppy
drive is unavailable.
Signed-off-by: Mark Cave-Ayland
---
hw/block/swim.c | 212 +---
hw/block/trace-events | 7 +-
include/hw/block/swim.h | 8 +-
3 files changed, 143 insertions(+), 84 deletions(-)
diff --git a/hw/block/swim.c b/hw/block/swim.c
index 735b335883..fd65c59f8a 100644
--- a/hw/block/swim.c
+++ b/hw/block/swim.c
@@ -21,24 +21,28 @@
#include "hw/qdev-properties.h"
#include "trace.h"
+
+/* IWM latch bits */
+
+#define IWMLB_PHASE00
+#define IWMLB_PHASE11
+#define IWMLB_PHASE22
+#define IWMLB_PHASE33
+#define IWMLB_MOTORON 4
+#define IWMLB_DRIVESEL 5
+#define IWMLB_L66
+#define IWMLB_L77
+
/* IWM registers */
-#define IWM_PH0L0
-#define IWM_PH0H1
-#define IWM_PH1L2
-#define IWM_PH1H3
-#define IWM_PH2L4
-#define IWM_PH2H5
-#define IWM_PH3L6
-#define IWM_PH3H7
-#define IWM_MTROFF 8
-#define IWM_MTRON 9
-#define IWM_INTDRIVE10
-#define IWM_EXTDRIVE11
-#define IWM_Q6L 12
-#define IWM_Q6H 13
-#define IWM_Q7L 14
-#define IWM_Q7H 15
+#define IWM_READALLONES 0
+#define IWM_READDATA1
+#define IWM_READSTATUS0 2
+#define IWM_READSTATUS1 3
+#define IWM_READWHANDSHAKE0 4
+#define IWM_READWHANDSHAKE1 5
+#define IWM_WRITESETMODE6
+#define IWM_WRITEDATA 7
/* SWIM registers */
@@ -62,8 +66,9 @@
#define REG_SHIFT 9
-#define SWIM_MODE_IWM 0
-#define SWIM_MODE_SWIM 1
+#define SWIM_MODE_STATUS_BIT6
+#define SWIM_MODE_IWM 0
+#define SWIM_MODE_ISM 1
/* bits in phase register */
@@ -127,10 +132,8 @@
#define SWIM_MOTON 0x80
static const char *iwm_reg_names[] = {
-"PH0L", "PH0H", "PH1L", "PH1H",
-"PH2L", "PH2H", "PH3L", "PH3H",
-"MTROFF", "MTRON", "INTDRIVE", "EXTDRIVE",
-"Q6L", "Q6H", "Q7L", "Q7H"
+"READALLONES", "READDATA", "READSTATUS0", "READSTATUS1",
+"READWHANDSHAKE0", "READWHANDSHAKE1", "WRITESETMODE", "WRITEDATA"
};
static const char *ism_reg_names[] = {
@@ -274,68 +277,99 @@ static const TypeInfo swim_bus_info = {
.instance_size = sizeof(SWIMBus),
};
-static void iwmctrl_write(void *opaque, hwaddr reg, uint64_t value,
+static void iwmctrl_write(void *opaque, hwaddr addr, uint64_t value,
unsigned size)
{
SWIMCtrl *swimctrl = opaque;
+uint8_t latch, reg, ism_bit;
-reg >>= REG_SHIFT;
+addr >>= REG_SHIFT;
+
+/* A3-A1 select a latch, A0 specifies the value */
+latch = (addr >> 1) & 7;
+if (addr & 1) {
+swimctrl->iwm_latches |= (1 << latch);
+} else {
+swimctrl->iwm_latches &= ~(1 << latch);
+}
+
+reg = (swimctrl->iwm_latches & 0xc0) >> 5 |
+ (swimctrl->iwm_latches & 0x10) >> 4;
swimctrl->iwmregs[reg] = value;
trace_swim_iwmctrl_write(reg, iwm_reg_names[reg], size, value);
-if (swimctrl->iwmregs[IWM_Q7H]) {
-if (swimctrl->iwmregs[IWM_MTRON]) {
-/* data register */
-swimctrl->iwm_data = value;
-} else {
-/* mode register */
-swimctrl->iwm_mode = value;
-/* detect sequence to switch from IWM mode to SWIM mode */
-switch (swimctrl->iwm_switch) {
-case 0:
-if (value == 0x57) {
-swimctrl->iwm_switch++;
-}
-break;
-case 1:
-if (value == 0x17) {
-swimctrl->iwm_switch++;
-}
-break;
-case 2:
-if (value == 0x57) {
-swimctrl->iwm_switch++;
-}
-break;
-case 3:
-if (value == 0x57) {
-swimctrl->mode = SWIM_MODE_SWIM;
-swimctrl->iwm_switch = 0;
-trace_swim_iwm_switch();
-
-/* Switch to ISM registers */
-memory_region_del_subregion(&swimctrl->swim,
-&swimctrl->iwm);
-memory_region_add_subregion(&swimctrl->swim, 0x0,
-&swimctrl->ism);
-}
-break;
+switch (reg) {
+case IWM_WRITESETMODE:
+/* detect sequence to switch from IWM mode to SWIM mode */
+ism_bit = (value & (1 << SWIM_MODE_STATUS_BIT));
+
+switch (swim
[PATCH 17/30] q800: add easc bool machine class property to switch between ASC and EASC
This determines whether the Apple Sound Chip (ASC) is set to enhanced mode
(default) or to original mode. The real Q800 hardware used an EASC chip however
a lot of older software only works with the older ASC chip.
Adding this as a machine parameter allows QEMU to be used as an developer aid
for testing and migrating code from ASC to EASC.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 30 +-
include/hw/m68k/q800.h | 1 +
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index ed862f9e9d..1af1a06f64 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -710,7 +710,8 @@ static void q800_machine_init(MachineState *machine)
/* Apple Sound Chip */
dev = qdev_new(TYPE_ASC);
-qdev_prop_set_uint8(dev, "asctype", ASC_TYPE_EASC);
+qdev_prop_set_uint8(dev, "asctype", m->easc ? ASC_TYPE_EASC
+: ASC_TYPE_ASC);
sysbus = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
memory_region_add_subregion(&m->macio, ASC_BASE - IO_BASE,
@@ -886,6 +887,28 @@ static void q800_machine_init(MachineState *machine)
}
}
+static bool q800_get_easc(Object *obj, Error **errp)
+{
+Q800MachineState *ms = Q800_MACHINE(obj);
+
+return ms->easc;
+}
+
+static void q800_set_easc(Object *obj, bool value, Error **errp)
+{
+Q800MachineState *ms = Q800_MACHINE(obj);
+
+ms->easc = value;
+}
+
+static void q800_init(Object *obj)
+{
+Q800MachineState *ms = Q800_MACHINE(obj);
+
+/* Default to EASC */
+ms->easc = true;
+}
+
static GlobalProperty hw_compat_q800[] = {
{ "scsi-hd", "quirk_mode_page_vendor_specific_apple", "on" },
{ "scsi-hd", "vendor", " SEAGATE" },
@@ -912,11 +935,16 @@ static void q800_machine_class_init(ObjectClass *oc, void
*data)
mc->block_default_type = IF_SCSI;
mc->default_ram_id = "m68k_mac.ram";
compat_props_add(mc->compat_props, hw_compat_q800, hw_compat_q800_len);
+
+object_class_property_add_bool(oc, "easc", q800_get_easc, q800_set_easc);
+object_class_property_set_description(oc, "easc",
+"Set to off to use ASC rather than EASC");
}
static const TypeInfo q800_machine_typeinfo = {
.name = MACHINE_TYPE_NAME("q800"),
.parent = TYPE_MACHINE,
+.instance_init = q800_init,
.instance_size = sizeof(Q800MachineState),
.class_init = q800_machine_class_init,
};
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 0602d07d3d..0144be5e6e 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -30,6 +30,7 @@
struct Q800MachineState {
MachineState parent_obj;
+bool easc;
M68kCPU *cpu;
MemoryRegion rom;
DeviceState *glue;
--
2.30.2
[PATCH 21/30] mac_via: work around underflow in TimeDBRA timing loop in SETUPTIMEK
The MacOS toolbox ROM calculates the number of branches that can be executed
per millisecond as part of its timer calibration. Since modern hosts are
considerably quicker than original hardware, the negative counter reaches zero
before the calibration completes leading to division by zero later in
CALCULATESLOD.
Instead of trying to fudge the timing loop (which won't work for
TimeDBRA/TimeSCCDB
anyhow), use the pattern of access to the VIA1 registers to detect when
SETUPTIMEK
has finished executing and write some well-known good timer values to TimeDBRA
and TimeSCCDB taken from real hardware with a suitable scaling factor.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 115 ++
hw/misc/trace-events | 1 +
include/hw/misc/mac_via.h | 3 +
3 files changed, 119 insertions(+)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index f90a22a067..62f0988537 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -16,6 +16,7 @@
*/
#include "qemu/osdep.h"
+#include "exec/address-spaces.h"
#include "migration/vmstate.h"
#include "hw/sysbus.h"
#include "hw/irq.h"
@@ -870,6 +871,112 @@ static void via1_auxmode_update(MOS6522Q800VIA1State *v1s)
}
}
+/*
+ * Addresses and real values for TimeDBRA/TimeSCCB to allow timer calibration
+ * to succeed (NOTE: both values have been multiplied by 3 to cope with the
+ * speed of QEMU execution on a modern host
+ */
+#define MACOS_TIMEDBRA0xd00
+#define MACOS_TIMESCCB0xd02
+
+#define MACOS_TIMEDBRA_VALUE (0x2a00 * 3)
+#define MACOS_TIMESCCB_VALUE (0x079d * 3)
+
+static bool via1_is_toolbox_timer_calibrated(void)
+{
+/*
+ * Indicate whether the MacOS toolbox has been calibrated by checking
+ * for the value of our magic constants
+ */
+uint16_t timedbra = lduw_be_phys(&address_space_memory, MACOS_TIMEDBRA);
+uint16_t timesccdb = lduw_be_phys(&address_space_memory, MACOS_TIMESCCB);
+
+return (timedbra == MACOS_TIMEDBRA_VALUE &&
+timesccdb == MACOS_TIMESCCB_VALUE);
+}
+
+static void via1_timer_calibration_hack(MOS6522Q800VIA1State *v1s, int addr,
+uint8_t val)
+{
+/*
+ * Work around timer calibration to ensure we that we have non-zero and
+ * known good values for TIMEDRBA and TIMESCCDB.
+ *
+ * This works by attempting to detect the reset and calibration sequence
+ * of writes to VIA1
+ */
+int old_timer_hack_state = v1s->timer_hack_state;
+
+switch (v1s->timer_hack_state) {
+case 0:
+if (addr == VIA_REG_PCR && val == 0x22) {
+/* VIA_REG_PCR: configure VIA1 edge triggering */
+v1s->timer_hack_state = 1;
+}
+break;
+case 1:
+if (addr == VIA_REG_T2CL && val == 0xc) {
+/* VIA_REG_T2CL: low byte of 1ms counter */
+if (!via1_is_toolbox_timer_calibrated()) {
+v1s->timer_hack_state = 2;
+} else {
+v1s->timer_hack_state = 0;
+}
+}
+break;
+case 2:
+if (addr == VIA_REG_T2CH && val == 0x3) {
+/*
+ * VIA_REG_T2CH: high byte of 1ms counter (very likely at the
+ * start of SETUPTIMEK)
+ */
+if (!via1_is_toolbox_timer_calibrated()) {
+v1s->timer_hack_state = 3;
+} else {
+v1s->timer_hack_state = 0;
+}
+}
+break;
+case 3:
+if (addr == VIA_REG_IER && val == 0x20) {
+/*
+ * VIA_REG_IER: update at end of SETUPTIMEK
+ *
+ * Timer calibration has finished: unfortunately the values in
+ * TIMEDBRA (0xd00) and TIMESCCDB (0xd02) are so far out they
+ * cause divide by zero errors.
+ *
+ * Update them with values obtained from a real Q800 but with
+ * a x3 scaling factor which seems to work well
+ */
+stw_be_phys(&address_space_memory, MACOS_TIMEDBRA,
+MACOS_TIMEDBRA_VALUE);
+stw_be_phys(&address_space_memory, MACOS_TIMESCCB,
+MACOS_TIMESCCB_VALUE);
+
+v1s->timer_hack_state = 4;
+}
+break;
+case 4:
+/*
+ * This is the normal post-calibration timer state: we should
+ * generally remain here unless we detect the A/UX calibration
+ * loop, or a write to VIA_REG_PCR suggesting a reset
+ */
+if (addr == VIA_REG_PCR && val == 0x22) {
+/* Looks like there has been a reset? */
+v1s->timer_hack_state = 1;
+}
+break;
+default:
+g_assert_not_reached();
+}
+
+if (old_timer_hack_state != v1s->timer_hack_state) {
+trace_via1_timer_hack_state(v1s->timer_hack_state);
+}
+}
+
static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned
size)
{
[PATCH 09/30] q800: add djMEMC memory controller
The djMEMC controller is used to store information related to the physical memory configuration. Co-developed-by: Laurent Vivier Signed-off-by: Mark Cave-Ayland --- MAINTAINERS | 2 + hw/m68k/Kconfig | 1 + hw/m68k/q800.c | 9 +++ hw/misc/Kconfig | 3 + hw/misc/djmemc.c | 154 +++ hw/misc/meson.build | 1 + hw/misc/trace-events | 4 + include/hw/m68k/q800.h | 2 + include/hw/misc/djmemc.h | 46 9 files changed, 222 insertions(+) create mode 100644 hw/misc/djmemc.c create mode 100644 include/hw/misc/djmemc.h diff --git a/MAINTAINERS b/MAINTAINERS index 86a1b88863..21ec70d00a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1227,6 +1227,7 @@ F: hw/misc/mac_via.c F: hw/nubus/* F: hw/display/macfb.c F: hw/block/swim.c +F: hw/misc/djmemc.c F: hw/m68k/bootinfo.h F: include/standard-headers/asm-m68k/bootinfo.h F: include/standard-headers/asm-m68k/bootinfo-mac.h @@ -1235,6 +1236,7 @@ F: include/hw/nubus/* F: include/hw/display/macfb.h F: include/hw/block/swim.h F: include/hw/m68k/q800.h +F: include/hw/misc/djmemc.c virt M: Laurent Vivier diff --git a/hw/m68k/Kconfig b/hw/m68k/Kconfig index f839f8a030..330cfdfa2d 100644 --- a/hw/m68k/Kconfig +++ b/hw/m68k/Kconfig @@ -23,6 +23,7 @@ config Q800 select ESP select DP8393X select OR_IRQ +select DJMEMC config M68K_VIRT bool diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c index f15f1eaff9..456407898e 100644 --- a/hw/m68k/q800.c +++ b/hw/m68k/q800.c @@ -40,6 +40,7 @@ #include "bootinfo.h" #include "hw/m68k/q800.h" #include "hw/misc/mac_via.h" +#include "hw/misc/djmemc.h" #include "hw/input/adb.h" #include "hw/nubus/mac-nubus-bridge.h" #include "hw/display/macfb.h" @@ -66,6 +67,7 @@ #define SONIC_PROM_BASE (IO_BASE + 0x08000) #define SONIC_BASE(IO_BASE + 0x0a000) #define SCC_BASE (IO_BASE + 0x0c020) +#define DJMEMC_BASE (IO_BASE + 0x0e000) #define ESP_BASE (IO_BASE + 0x1) #define ESP_PDMA (IO_BASE + 0x10100) #define ASC_BASE (IO_BASE + 0x14000) @@ -492,6 +494,13 @@ static void q800_machine_init(MachineState *machine) &error_abort); sysbus_realize_and_unref(SYS_BUS_DEVICE(m->glue), &error_fatal); +/* djMEMC memory controller */ +m->djmemc = qdev_new(TYPE_DJMEMC); +sysbus = SYS_BUS_DEVICE(m->djmemc); +sysbus_realize_and_unref(sysbus, &error_fatal); +memory_region_add_subregion(&m->macio, DJMEMC_BASE - IO_BASE, +sysbus_mmio_get_region(sysbus, 0)); + /* VIA 1 */ via1_dev = qdev_new(TYPE_MOS6522_Q800_VIA1); dinfo = drive_get(IF_MTD, 0, 0); diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig index 2ef5781ef8..7eaf955f88 100644 --- a/hw/misc/Kconfig +++ b/hw/misc/Kconfig @@ -180,4 +180,7 @@ config AXP209_PMU bool depends on I2C +config DJMEMC +bool + source macio/Kconfig diff --git a/hw/misc/djmemc.c b/hw/misc/djmemc.c new file mode 100644 index 00..597e0d446c --- /dev/null +++ b/hw/misc/djmemc.c @@ -0,0 +1,154 @@ +/* + * djMEMC, macintosh memory and interrupt controller + * (Quadra 610/650/800 & Centris 610/650) + * + *https://mac68k.info/wiki/display/mac68k/djMEMC+Information + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "migration/vmstate.h" +#include "hw/misc/djmemc.h" +#include "hw/qdev-properties.h" +#include "trace.h" + + +#define DJMEMC_INTERLEAVECONF 0x0 +#define DJMEMC_BANK0CONF0x4 +#define DJMEMC_BANK1CONF0x8 +#define DJMEMC_BANK2CONF0xc +#define DJMEMC_BANK3CONF0x10 +#define DJMEMC_BANK4CONF0x14 +#define DJMEMC_BANK5CONF0x18 +#define DJMEMC_BANK6CONF0x1c +#define DJMEMC_BANK7CONF0x20 +#define DJMEMC_BANK8CONF0x24 +#defi
[PATCH 28/30] q800: add alias for MacOS toolbox ROM at 0x40000000
According to the Apple Quadra 800 Developer Note document, the Quadra 800 ROM
consists of 2 ROM code sections based at offsets 0x0 and 0x80. A/UX attempts
to access the toolbox ROM at the lower offset during startup, so provide a
memory alias to allow the access to succeed.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 5 +
include/hw/m68k/q800.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 3acdb5dd8d..bf4acb5db7 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -869,6 +869,11 @@ static void q800_machine_init(MachineState *machine)
filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
memory_region_add_subregion(get_system_memory(), MACROM_ADDR, &m->rom);
+memory_region_init_alias(&m->rom_alias, NULL, "m68k_mac.rom-alias",
+ &m->rom, 0, MACROM_SIZE);
+memory_region_add_subregion(get_system_memory(), 0x4000,
+&m->rom_alias);
+
/* Load MacROM binary */
if (filename) {
bios_size = load_image_targphys(filename, MACROM_ADDR,
MACROM_SIZE);
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 3039b24d30..1a0dbb5ecd 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -33,6 +33,7 @@ struct Q800MachineState {
bool easc;
M68kCPU *cpu;
MemoryRegion rom;
+MemoryRegion rom_alias;
DeviceState *glue;
DeviceState *djmemc;
MemoryRegion ramio;
--
2.30.2
[PATCH 08/30] q800: reimplement mac-io region aliasing using IO memory region
The current use of aliased memory regions causes us 2 problems: firstly the
output of "info qom-tree" is absolutely huge and difficult to read, and
secondly we have already reached the internal limit for memory regions as
adding any new memory region into the mac-io region causes QEMU to assert
with "phys_section_add: Assertion `map->sections_nb < TARGET_PAGE_SIZE'
failed".
Implement the mac-io region aliasing using a single IO memory region that
applies IO_SLICE_MASK representing the maximum size of the aliased region and
then forwarding the access to the existing mac-io memory region using the
address space API.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 100 +
include/hw/m68k/q800.h | 1 +
2 files changed, 82 insertions(+), 19 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 6399631ed0..f15f1eaff9 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -59,6 +59,7 @@
#define IO_BASE 0x5000
#define IO_SLICE 0x0004
+#define IO_SLICE_MASK (IO_SLICE - 1)
#define IO_SIZE 0x0400
#define VIA_BASE (IO_BASE + 0x0)
@@ -361,6 +362,68 @@ static uint8_t fake_mac_rom[] = {
0x60, 0xFE /* bras [self] */
};
+static MemTxResult macio_alias_read(void *opaque, hwaddr addr, uint64_t *data,
+unsigned size, MemTxAttrs attrs)
+{
+MemTxResult r;
+uint32_t val;
+
+addr &= IO_SLICE_MASK;
+addr |= IO_BASE;
+
+switch (size) {
+case 4:
+val = address_space_ldl_be(&address_space_memory, addr, attrs, &r);
+break;
+case 2:
+val = address_space_lduw_be(&address_space_memory, addr, attrs, &r);
+break;
+case 1:
+val = address_space_ldub(&address_space_memory, addr, attrs, &r);
+break;
+default:
+g_assert_not_reached();
+}
+
+*data = val;
+return r;
+}
+
+static MemTxResult macio_alias_write(void *opaque, hwaddr addr, uint64_t value,
+ unsigned size, MemTxAttrs attrs)
+{
+MemTxResult r;
+
+addr &= IO_SLICE_MASK;
+addr |= IO_BASE;
+
+switch (size) {
+case 4:
+address_space_stl_be(&address_space_memory, addr, value, attrs, &r);
+break;
+case 2:
+address_space_stw_be(&address_space_memory, addr, value, attrs, &r);
+break;
+case 1:
+address_space_stb(&address_space_memory, addr, value, attrs, &r);
+break;
+default:
+g_assert_not_reached();
+}
+
+return r;
+}
+
+static const MemoryRegionOps macio_alias_ops = {
+.read_with_attrs = macio_alias_read,
+.write_with_attrs = macio_alias_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 4,
+},
+};
+
static void q800_machine_init(MachineState *machine)
{
Q800MachineState *m = Q800_MACHINE(machine);
@@ -371,10 +434,8 @@ static void q800_machine_init(MachineState *machine)
int bios_size;
ram_addr_t initrd_base;
int32_t initrd_size;
-MemoryRegion *io;
MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1);
uint8_t *prom;
-const int io_slice_nb = (IO_SIZE / IO_SLICE) - 1;
int i, checksum;
MacFbMode *macfb_mode;
ram_addr_t ram_size = machine->ram_size;
@@ -420,16 +481,10 @@ static void q800_machine_init(MachineState *machine)
* Memory from IO_BASE to IO_BASE + IO_SLICE is repeated
* from IO_BASE + IO_SLICE to IO_BASE + IO_SIZE
*/
-io = g_new(MemoryRegion, io_slice_nb);
-for (i = 0; i < io_slice_nb; i++) {
-char *name = g_strdup_printf("mac_m68k.io[%d]", i + 1);
-
-memory_region_init_alias(&io[i], NULL, name, get_system_memory(),
- IO_BASE, IO_SLICE);
-memory_region_add_subregion(get_system_memory(),
-IO_BASE + (i + 1) * IO_SLICE, &io[i]);
-g_free(name);
-}
+memory_region_init_io(&m->macio_alias, NULL, &macio_alias_ops, &m->macio,
+ "mac-io.alias", IO_SIZE - IO_SLICE);
+memory_region_add_subregion(get_system_memory(), IO_BASE + IO_SLICE,
+&m->macio_alias);
/* IRQ Glue */
m->glue = qdev_new(TYPE_GLUE);
@@ -445,7 +500,8 @@ static void q800_machine_init(MachineState *machine)
}
sysbus = SYS_BUS_DEVICE(via1_dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
-sysbus_mmio_map(sysbus, 1, VIA_BASE);
+memory_region_add_subregion(&m->macio, VIA_BASE - IO_BASE,
+sysbus_mmio_get_region(sysbus, 1));
sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(m->glue, GLUE_IRQ_IN_VIA1));
/* A/UX mode */
qdev_connect_gpio_out(via1_dev, 0,
@@ -461,7 +517,8 @@ static void q800_machine_init(MachineState *machine)
via2_dev = qdev_new(TYPE_MOS6522_Q800_VIA2);
sysbus =
[PATCH 22/30] mac_via: fix rtc command decoding from PRAM addresses 0x0 to 0xf
A comparison between the rtc command table included in the comment and the code
itself shows that the decoding for PRAM addresses 0x0 to 0xf is being done on
the raw command, and not the shifted version held in value.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 62f0988537..d7067030db 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -403,7 +403,7 @@ static int via1_rtc_compact_cmd(uint8_t value)
} else if ((value & 0x1c) == 0x08) {
/* RAM address 0x10 to 0x13 */
return read | (REG_PRAM_ADDR + 0x10 + (value & 0x03));
-} else if ((value & 0x43) == 0x41) {
+} else if ((value & 0x10) == 0x10) {
/* RAM address 0x00 to 0x0f */
return read | (REG_PRAM_ADDR + (value & 0x0f));
}
--
2.30.2
[PATCH 24/30] mac_via: workaround NetBSD ADB bus enumeration issue
NetBSD assumes it can send its first ADB command after sending the ADB_BUSRESET
command in ADB_STATE_NEW without changing the state back to ADB_STATE_IDLE
first as detailed in the ADB protocol.
Add a workaround to detect this condition at the start of ADB enumeration
and send the next command written to SR after a ADB_BUSRESET onto the bus
regardless, even if we don't detect a state transition to ADB_STATE_NEW.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c| 34 ++
hw/misc/trace-events | 1 +
2 files changed, 35 insertions(+)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 5d5334b0f6..564db8337e 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -1001,6 +1001,8 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr
addr, uint64_t val,
{
MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque);
MOS6522State *ms = MOS6522(v1s);
+int oldstate, state;
+int oldsr = ms->sr;
addr = (addr >> 9) & 0xf;
@@ -1016,6 +1018,38 @@ static void mos6522_q800_via1_write(void *opaque, hwaddr
addr, uint64_t val,
v1s->last_b = ms->b;
break;
+
+case VIA_REG_SR:
+{
+/*
+ * NetBSD assumes it can send its first ADB command after sending
+ * the ADB_BUSRESET command in ADB_STATE_NEW without changing the
+ * state back to ADB_STATE_IDLE first as detailed in the ADB
+ * protocol.
+ *
+ * Add a workaround to detect this condition at the start of ADB
+ * enumeration and send the next command written to SR after a
+ * ADB_BUSRESET onto the bus regardless, even if we don't detect a
+ * state transition to ADB_STATE_NEW.
+ *
+ * Note that in my tests the NetBSD state machine takes one ADB
+ * operation to recover which means the probe for an ADB device at
+ * address 1 always fails. However since the first device is at
+ * address 2 then this will work fine, without having to come up
+ * with a more complicated and invasive solution.
+ */
+oldstate = (v1s->last_b & VIA1B_vADB_StateMask) >>
+ VIA1B_vADB_StateShift;
+state = (ms->b & VIA1B_vADB_StateMask) >> VIA1B_vADB_StateShift;
+
+if (oldstate == ADB_STATE_NEW && state == ADB_STATE_NEW &&
+(ms->acr & VIA1ACR_vShiftOut) &&
+oldsr == 0 /* ADB_BUSRESET */) {
+trace_via1_adb_netbsd_enum_hack();
+adb_via_send(v1s, state, ms->sr);
+}
+}
+break;
}
}
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index d3a9295d2f..7206bd5d93 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -249,6 +249,7 @@ via1_rtc_cmd_pram_sect_write(int sector, int offset, int
addr, int value) "secto
via1_adb_send(const char *state, uint8_t data, const char *vadbint) "state %s
data=0x%02x vADBInt=%s"
via1_adb_receive(const char *state, uint8_t data, const char *vadbint, int
status, int index, int size) "state %s data=0x%02x vADBInt=%s status=0x%x
index=%d size=%d"
via1_adb_poll(uint8_t data, const char *vadbint, int status, int index, int
size) "data=0x%02x vADBInt=%s status=0x%x index=%d size=%d"
+via1_adb_netbsd_enum_hack(void) "using NetBSD enum hack"
via1_auxmode(int mode) "setting auxmode to %d"
via1_timer_hack_state(int state) "setting timer_hack_state to %d"
--
2.30.2
[PATCH 23/30] mac_via: fix rtc command decoding for the PRAM seconds registers
Analysis of the MacOS toolbox ROM code shows that on startup it attempts 2
separate reads of the seconds registers with commands 0x9d...0x91 followed by
0x8d..0x81 without resetting the command to its initial value. The PRAM seconds
value is only accepted when the values of the 2 separate reads match.
>From this we conclude that bit 4 of the rtc command is not decoded or we don't
care about its value when reading the PRAM seconds registers. Implement this
decoding change so that both reads return successfully which allows the MacOS
toolbox ROM to correctly set the date/time.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index d7067030db..5d5334b0f6 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -366,10 +366,10 @@ static void pram_update(MOS6522Q800VIA1State *v1s)
*
* Command byteRegister addressed by the command
*
- * z001Seconds register 0 (lowest-order byte)
- * z101Seconds register 1
- * z0001001Seconds register 2
- * z0001101Seconds register 3 (highest-order byte)
+ * z00x0001Seconds register 0 (lowest-order byte)
+ * z00x0101Seconds register 1
+ * z00x1001Seconds register 2
+ * z00x1101Seconds register 3 (highest-order byte)
* 00110001Test register (write-only)
* 00110101Write-Protect Register (write-only)
* z010aa01RAM address 100aa ($10-$13) (first 20 bytes only)
@@ -377,6 +377,7 @@ static void pram_update(MOS6522Q800VIA1State *v1s)
* z0111aaaExtended memory designator and sector number
*
* For a read request, z=1, for a write z=0
+ * The letter x indicates don't care
* The letter a indicates bits whose value depend on what parameter
* RAM byte you want to address
*/
@@ -393,7 +394,7 @@ static int via1_rtc_compact_cmd(uint8_t value)
}
if ((value & 0x03) == 0x01) {
value >>= 2;
-if ((value & 0x1c) == 0) {
+if ((value & 0x18) == 0) {
/* seconds registers */
return read | (REG_0 + (value & 0x03));
} else if ((value == 0x0c) && !read) {
--
2.30.2
[PATCH 30/30] mac_via: work around QEMU unaligned MMIO access bug
During the kernel timer calibration routine A/UX performs an unaligned access
across the T1CL and T1CH registers to read the entire 16-bit value in a
single memory access.
This triggers a bug in the QEMU softtlb implementation whereby the 2 separate
accesses are combined incorrectly losing the high byte of the counter (see
https://gitlab.com/qemu-project/qemu/-/issues/360 for more detail). Since
A/UX requires a minimum difference of 0x500 between 2 subsequent reads to
succeed then this causes the timer calibration routine to get stuck in an
infinite loop.
Add a temporary workaround for the QEMU unaligned MMIO access bug whereby
these special accesses are detected and the 8-byte result copied into both
halves of the 16-bit access which allows the existing softtlb implementation
to return the correct result.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c| 1 +
hw/misc/mac_via.c | 42 +++
hw/misc/trace-events | 1 +
include/hw/misc/mac_via.h | 4 +++-
4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index bf4acb5db7..918cc8f695 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -443,6 +443,7 @@ static const MemoryRegionOps macio_alias_ops = {
.valid = {
.min_access_size = 1,
.max_access_size = 4,
+.unaligned = true, /* For VIA1 via1_unaligned_hack_state() */
},
};
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 4ec1ee18dd..45c8dee9f4 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -1026,12 +1026,47 @@ static void
via1_timer_calibration_hack(MOS6522Q800VIA1State *v1s, int addr,
}
}
+static bool via1_unaligned_hack_state(MOS6522Q800VIA1State *v1s, hwaddr addr,
+ int size)
+{
+/*
+ * Workaround for bug in QEMU whereby load_helper() doesn't correctly
+ * handle combining unaligned memory accesses: see QEMU issue
+ * https://gitlab.com/qemu-project/qemu/-/issues/360 for all the
+ * details.
+ *
+ * Its only known use is during the A/UX timer calibration loop which
+ * runs on kernel startup.
+ */
+switch (v1s->unaligned_hack_state) {
+case 0:
+/* First half of unaligned access */
+if (addr == 0x11fe && size == 2) {
+v1s->unaligned_hack_state = 1;
+trace_via1_unaligned_hack_state(v1s->unaligned_hack_state);
+return true;
+}
+return false;
+case 1:
+/* Second half of unaligned access */
+if (addr == 0x1200 && size == 2) {
+v1s->unaligned_hack_state = 0;
+trace_via1_unaligned_hack_state(v1s->unaligned_hack_state);
+return true;
+}
+return false;
+default:
+g_assert_not_reached();
+}
+}
+
static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned
size)
{
MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque);
MOS6522State *ms = MOS6522(v1s);
int64_t now;
uint64_t ret;
+hwaddr oldaddr = addr;
addr = (addr >> 9) & 0xf;
ret = mos6522_read(ms, addr, size);
@@ -1059,6 +1094,12 @@ static uint64_t mos6522_q800_via1_read(void *opaque,
hwaddr addr, unsigned size)
}
break;
}
+
+if (via1_unaligned_hack_state(v1s, oldaddr, size)) {
+/* Splat return byte into word to fix unaligned access combine */
+ret |= ret << 8;
+}
+
return ret;
}
@@ -1126,6 +1167,7 @@ static const MemoryRegionOps mos6522_q800_via1_ops = {
.valid = {
.min_access_size = 1,
.max_access_size = 4,
+.unaligned = true, /* For VIA1 via1_unaligned_hack_state() */
},
};
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 7206bd5d93..8867cef356 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -252,6 +252,7 @@ via1_adb_poll(uint8_t data, const char *vadbint, int
status, int index, int size
via1_adb_netbsd_enum_hack(void) "using NetBSD enum hack"
via1_auxmode(int mode) "setting auxmode to %d"
via1_timer_hack_state(int state) "setting timer_hack_state to %d"
+via1_unaligned_hack_state(int state) "setting unaligned_hack_state to %d"
# grlib_ahb_apb_pnp.c
grlib_ahb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "AHB PnP read
addr:0x%03"PRIx64" size:%u data:0x%08x"
diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h
index 63cdcf7c69..0a12737552 100644
--- a/include/hw/misc/mac_via.h
+++ b/include/hw/misc/mac_via.h
@@ -77,8 +77,10 @@ struct MOS6522Q800VIA1State {
/* SETUPTIMEK hack */
int timer_hack_state;
-};
+/* Unaligned access hack */
+int unaligned_hack_state;
+};
/* VIA 2 */
#define VIA2_IRQ_SCSI_DATA_BIT CA2_INT_BIT
--
2.30.2
[PATCH 03/30] q800: rename q800_init() to q800_machine_init()
This will enable us later to distinguish between QOM initialisation and machine
initialisation.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index bdccd93c7f..976da06231 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -361,7 +361,7 @@ static uint8_t fake_mac_rom[] = {
0x60, 0xFE /* bras [self] */
};
-static void q800_init(MachineState *machine)
+static void q800_machine_init(MachineState *machine)
{
M68kCPU *cpu = NULL;
int linux_boot;
@@ -737,8 +737,9 @@ static const size_t hw_compat_q800_len =
G_N_ELEMENTS(hw_compat_q800);
static void q800_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
+
mc->desc = "Macintosh Quadra 800";
-mc->init = q800_init;
+mc->init = q800_machine_init;
mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
mc->max_cpus = 1;
mc->block_default_type = IF_SCSI;
--
2.30.2
[PATCH 29/30] mac_via: extend timer calibration hack to work with A/UX
The A/UX timer calibration loop runs continuously until 2 consecutive iterations
differ by at least 0x492 timer ticks. Modern hosts execute the timer calibration
loop so fast that this situation never occurs causing a hang on boot.
Use a similar method to Shoebill which is to randomly add 0x500 to the T2
counter value during calibration to enable it to eventually succeed.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 55 +--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index ee44cb4437..4ec1ee18dd 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -973,6 +973,7 @@ static void
via1_timer_calibration_hack(MOS6522Q800VIA1State *v1s, int addr,
v1s->timer_hack_state = 4;
}
break;
+
case 4:
/*
* This is the normal post-calibration timer state: we should
@@ -983,6 +984,38 @@ static void
via1_timer_calibration_hack(MOS6522Q800VIA1State *v1s, int addr,
/* Looks like there has been a reset? */
v1s->timer_hack_state = 1;
}
+
+if (addr == VIA_REG_T2CL && val == 0xf0) {
+/* VIA_REG_T2CH: high byte of counter (A/UX) */
+v1s->timer_hack_state = 5;
+}
+break;
+case 5:
+if (addr == VIA_REG_T2CH && val == 0x3c) {
+/* VIA_REG_T2CL: low byte of counter (A/UX) */
+v1s->timer_hack_state = 6;
+} else {
+v1s->timer_hack_state = 0;
+}
+break;
+case 6:
+if ((addr == VIA_REG_IER && val == 0x20) || addr == VIA_REG_T2CH) {
+/* End of A/UX timer calibration routine, or another write */
+v1s->timer_hack_state = 7;
+} else {
+v1s->timer_hack_state = 0;
+}
+break;
+case 7:
+/*
+ * This is the normal post-calibration timer state once both the
+ * MacOS toolbox and A/UX have been calibrated, until we see a write
+ * to VIA_REG_PCR to suggest a reset
+ */
+if (addr == VIA_REG_PCR && val == 0x22) {
+/* Looks like there has been a reset? */
+v1s->timer_hack_state = 1;
+}
break;
default:
g_assert_not_reached();
@@ -995,8 +1028,9 @@ static void
via1_timer_calibration_hack(MOS6522Q800VIA1State *v1s, int addr,
static uint64_t mos6522_q800_via1_read(void *opaque, hwaddr addr, unsigned
size)
{
-MOS6522Q800VIA1State *s = MOS6522_Q800_VIA1(opaque);
-MOS6522State *ms = MOS6522(s);
+MOS6522Q800VIA1State *v1s = MOS6522_Q800_VIA1(opaque);
+MOS6522State *ms = MOS6522(v1s);
+int64_t now;
uint64_t ret;
addr = (addr >> 9) & 0xf;
@@ -1007,6 +1041,23 @@ static uint64_t mos6522_q800_via1_read(void *opaque,
hwaddr addr, unsigned size)
/* Quadra 800 Id */
ret = (ret & ~VIA1A_CPUID_MASK) | VIA1A_CPUID_Q800;
break;
+case VIA_REG_T2CH:
+if (v1s->timer_hack_state == 6) {
+/*
+ * The A/UX timer calibration loop runs continuously until 2
+ * consecutive iterations differ by at least 0x492 timer ticks.
+ * Modern hosts execute the timer calibration loop so fast that
+ * this situation never occurs causing a hang on boot. Use a
+ * similar method to Shoebill which is to randomly add 0x500 to
+ * the T2 counter value during calibration to enable it to
+ * eventually succeed.
+ */
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+if (now & 1) {
+ret += 0x5;
+}
+}
+break;
}
return ret;
}
--
2.30.2
[PATCH 27/30] q800: add ESCC alias at 0xc000
Tests on real Q800 hardware show that the ESCC is addressable at multiple
locations
within the ESCC memory region - at least 0xc000, 0xc020 (as expected by the
MacOS
toolbox ROM) and 0xc040.
All released NetBSD kernels before 10 use the 0xc000 address which causes a
fatal
error when running the MacOS booter. Add a single memory region alias at 0xc000
to enable NetBSD kernels to start booting under QEMU.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 6 ++
include/hw/m68k/q800.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 1af1a06f64..3acdb5dd8d 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -682,6 +682,12 @@ static void q800_machine_init(MachineState *machine)
memory_region_add_subregion(&m->macio, SCC_BASE - IO_BASE,
sysbus_mmio_get_region(sysbus, 0));
+/* Create alias for NetBSD */
+memory_region_init_alias(&m->escc_alias, NULL, "escc-alias",
+ sysbus_mmio_get_region(sysbus, 0), 0, 0x8);
+memory_region_add_subregion(&m->macio, SCC_BASE - IO_BASE - 0x20,
+&m->escc_alias);
+
/* SCSI */
dev = qdev_new(TYPE_SYSBUS_ESP);
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 0144be5e6e..3039b24d30 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -40,6 +40,7 @@ struct Q800MachineState {
MemoryRegion macio;
MemoryRegion macio_alias;
MemoryRegion machine_id;
+MemoryRegion escc_alias;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
--
2.30.2
[PATCH 06/30] q800: move GLUE device to Q800MachineState
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 20 ++--
include/hw/m68k/q800.h | 1 +
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 6a000ceb75..c22a98d616 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -392,7 +392,6 @@ static void q800_machine_init(MachineState *machine)
SysBusDevice *sysbus;
BusState *adb_bus;
NubusBus *nubus;
-DeviceState *glue;
DriveInfo *dinfo;
uint8_t rng_seed[32];
@@ -427,9 +426,10 @@ static void q800_machine_init(MachineState *machine)
}
/* IRQ Glue */
-glue = qdev_new(TYPE_GLUE);
-object_property_set_link(OBJECT(glue), "cpu", OBJECT(m->cpu),
&error_abort);
-sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
+m->glue = qdev_new(TYPE_GLUE);
+object_property_set_link(OBJECT(m->glue), "cpu", OBJECT(m->cpu),
+ &error_abort);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(m->glue), &error_fatal);
/* VIA 1 */
via1_dev = qdev_new(TYPE_MOS6522_Q800_VIA1);
@@ -440,10 +440,10 @@ static void q800_machine_init(MachineState *machine)
sysbus = SYS_BUS_DEVICE(via1_dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
sysbus_mmio_map(sysbus, 1, VIA_BASE);
-sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, GLUE_IRQ_IN_VIA1));
+sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(m->glue, GLUE_IRQ_IN_VIA1));
/* A/UX mode */
qdev_connect_gpio_out(via1_dev, 0,
- qdev_get_gpio_in_named(glue, "auxmode", 0));
+ qdev_get_gpio_in_named(m->glue, "auxmode", 0));
adb_bus = qdev_get_child_bus(via1_dev, "adb.0");
dev = qdev_new(TYPE_ADB_KEYBOARD);
@@ -456,7 +456,7 @@ static void q800_machine_init(MachineState *machine)
sysbus = SYS_BUS_DEVICE(via2_dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
sysbus_mmio_map(sysbus, 1, VIA_BASE + VIA_SIZE);
-sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, GLUE_IRQ_IN_VIA2));
+sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(m->glue, GLUE_IRQ_IN_VIA2));
/* MACSONIC */
@@ -489,7 +489,7 @@ static void q800_machine_init(MachineState *machine)
sysbus = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(sysbus, &error_fatal);
sysbus_mmio_map(sysbus, 0, SONIC_BASE);
-sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(glue, GLUE_IRQ_IN_SONIC));
+sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(m->glue,
GLUE_IRQ_IN_SONIC));
memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom",
SONIC_PROM_SIZE, &error_fatal);
@@ -526,7 +526,7 @@ static void q800_machine_init(MachineState *machine)
sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(escc_orgate, 0));
sysbus_connect_irq(sysbus, 1, qdev_get_gpio_in(escc_orgate, 1));
qdev_connect_gpio_out(DEVICE(escc_orgate), 0,
- qdev_get_gpio_in(glue, GLUE_IRQ_IN_ESCC));
+ qdev_get_gpio_in(m->glue, GLUE_IRQ_IN_ESCC));
sysbus_mmio_map(sysbus, 0, SCC_BASE);
/* SCSI */
@@ -581,7 +581,7 @@ static void q800_machine_init(MachineState *machine)
* Since the framebuffer in slot 0x9 uses a separate IRQ, wire the unused
* IRQ via GLUE for use by SONIC Ethernet in classic mode
*/
-qdev_connect_gpio_out(glue, GLUE_IRQ_NUBUS_9,
+qdev_connect_gpio_out(m->glue, GLUE_IRQ_NUBUS_9,
qdev_get_gpio_in_named(via2_dev, "nubus-irq",
VIA2_NUBUS_IRQ_9));
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 2f3c720b8d..de02af53be 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -32,6 +32,7 @@ struct Q800MachineState {
M68kCPU *cpu;
MemoryRegion rom;
+DeviceState *glue;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
--
2.30.2
[PATCH 13/30] q800: allow accesses to RAM area even if less memory is available
MacOS attempts a series of writes and reads over the entire RAM area in order
to determine the amount of RAM within the machine. Allow accesses to the
entire RAM area ignoring writes and always reading zero for areas where there
is no physical RAM installed to allow MacOS to detect the memory size without
faulting.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 30 +-
include/hw/m68k/q800.h | 1 +
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 8310670ec2..d12aeaa20e 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -86,6 +86,9 @@
#define MAC_CLOCK 3686418
+/* Size of whole RAM area */
+#define RAM_SIZE 0x4000
+
/*
* Slot 0x9 is reserved for use by the in-built framebuffer whilst only
* slots 0xc, 0xd and 0xe physically exist on the Quadra 800
@@ -452,6 +455,27 @@ static const MemoryRegionOps machine_id_ops = {
},
};
+static uint64_t ramio_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0x0;
+}
+
+static void ramio_write(void *opaque, hwaddr addr, uint64_t val,
+unsigned size)
+{
+return;
+}
+
+static const MemoryRegionOps ramio_ops = {
+.read = ramio_read,
+.write = ramio_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 4,
+},
+};
+
static void q800_machine_init(MachineState *machine)
{
Q800MachineState *m = Q800_MACHINE(machine);
@@ -497,7 +521,11 @@ static void q800_machine_init(MachineState *machine)
qemu_register_reset(main_cpu_reset, m->cpu);
/* RAM */
-memory_region_add_subregion(get_system_memory(), 0, machine->ram);
+memory_region_init_io(&m->ramio, NULL, &ramio_ops, &m->ramio,
+ "ram", RAM_SIZE);
+memory_region_add_subregion(get_system_memory(), 0x0, &m->ramio);
+
+memory_region_add_subregion(&m->ramio, 0, machine->ram);
/*
* Create container for all IO devices
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index e57aec849a..0602d07d3d 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -34,6 +34,7 @@ struct Q800MachineState {
MemoryRegion rom;
DeviceState *glue;
DeviceState *djmemc;
+MemoryRegion ramio;
MemoryRegion macio;
MemoryRegion macio_alias;
--
2.30.2
[PATCH 26/30] mac_via: always clear ADB interrupt when switching to A/UX mode
When the NetBSD kernel initialises it can leave the ADB interrupt asserted
depending upon where in the ADB poll cycle the MacOS ADB interrupt handler
is when the NetBSD kernel disables interrupts.
The NetBSD ADB driver uses the ADB interrupt state to determine if the ADB
is busy and refuses to send ADB commands unless it is clear. To ensure that
this doesn't happen, always clear the ADB interrupt when switching to A/UX
mode to ensure that the bus enumeration always occurs.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index c1d2866ec9..ee44cb4437 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -875,6 +875,15 @@ static void via1_auxmode_update(MOS6522Q800VIA1State *v1s)
if (irq != oldirq) {
trace_via1_auxmode(irq);
qemu_set_irq(v1s->auxmode_irq, irq);
+
+/*
+ * Clear the ADB interrupt. MacOS can leave VIA1B_vADBInt asserted
+ * (low) if a poll sequence doesn't complete before NetBSD disables
+ * interrupts upon boot. Fortunately NetBSD switches to the so-called
+ * "A/UX" interrupt mode after it initialises, so we can use this as
+ * a convenient place to clear the ADB interrupt for now.
+ */
+s->b |= VIA1B_vADBInt;
}
}
--
2.30.2
[PATCH 14/30] audio: add Apple Sound Chip (ASC) emulation
The Apple Sound Chip was primarily used by the Macintosh II to generate sound
in hardware which was previously handled by the toolbox ROM with software
interrupts.
Implement both the standard ASC and also the enhanced ASC (EASC) functionality
which is used in the Quadra 800.
Note that whilst real ASC hardware uses AUDIO_FORMAT_S8, this implementation
uses
AUDIO_FORMAT_U8 instead because AUDIO_FORMAT_S8 is rarely used and not supported
by some audio backends like PulseAudio and DirectSound when played directly with
-audiodev out.mixing-engine=off.
Co-developed-by: Laurent Vivier
Co-developed-by: Volker Rümelin
Signed-off-by: Mark Cave-Ayland
---
MAINTAINERS| 2 +
hw/audio/Kconfig | 3 +
hw/audio/asc.c | 688 +
hw/audio/meson.build | 1 +
hw/audio/trace-events | 10 +
hw/m68k/Kconfig| 1 +
include/hw/audio/asc.h | 75 +
7 files changed, 780 insertions(+)
create mode 100644 hw/audio/asc.c
create mode 100644 include/hw/audio/asc.h
diff --git a/MAINTAINERS b/MAINTAINERS
index f151aaf99f..1b79ab7965 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1229,6 +1229,7 @@ F: hw/display/macfb.c
F: hw/block/swim.c
F: hw/misc/djmemc.c
F: hw/misc/iosb.c
+F: hw/audio/asc.c
F: hw/m68k/bootinfo.h
F: include/standard-headers/asm-m68k/bootinfo.h
F: include/standard-headers/asm-m68k/bootinfo-mac.h
@@ -1239,6 +1240,7 @@ F: include/hw/block/swim.h
F: include/hw/m68k/q800.h
F: include/hw/misc/djmemc.c
F: include/hw/misc/iosb.c
+F: include/hw/audio/asc.h
virt
M: Laurent Vivier
diff --git a/hw/audio/Kconfig b/hw/audio/Kconfig
index e76c69ca7e..d0993514a1 100644
--- a/hw/audio/Kconfig
+++ b/hw/audio/Kconfig
@@ -47,3 +47,6 @@ config PL041
config CS4231
bool
+
+config ASC
+bool
diff --git a/hw/audio/asc.c b/hw/audio/asc.c
new file mode 100644
index 00..04194b1e43
--- /dev/null
+++ b/hw/audio/asc.c
@@ -0,0 +1,688 @@
+/*
+ * QEMU Apple Sound Chip emulation
+ *
+ * Apple Sound Chip (ASC) 344S0063
+ * Enhanced Apple Sound Chip (EASC) 343S1063
+ *
+ * Copyright (c) 2012-2018 Laurent Vivier
+ * Copyright (c) 2022 Mark Cave-Ayland
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "audio/audio.h"
+#include "hw/audio/asc.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+/*
+ * Linux doesn't provide information about ASC, see arch/m68k/mac/macboing.c
+ * and arch/m68k/include/asm/mac_asc.h
+ *
+ * best information is coming from MAME:
+ * https://github.com/mamedev/mame/blob/master/src/devices/sound/asc.h
+ * https://github.com/mamedev/mame/blob/master/src/devices/sound/asc.cpp
+ * Emulation by R. Belmont
+ * or MESS:
+ * http://mess.redump.net/mess/driver_info/easc
+ *
+ * 0x800: VERSION
+ * 0x801: MODE
+ *1=FIFO mode,
+ *2=wavetable mode
+ * 0x802: CONTROL
+ *bit 0=analog or PWM output,
+ *1=stereo/mono,
+ *7=processing time exceeded
+ * 0x803: FIFO MODE
+ *bit 7=clear FIFO,
+ *bit 1="non-ROM companding",
+ *bit 0="ROM companding")
+ * 0x804: FIFO IRQ STATUS
+ *bit 0=ch A 1/2 full,
+ *1=ch A full,
+ *2=ch B 1/2 full,
+ *3=ch B full)
+ * 0x805: WAVETABLE CONTROL
+ *bits 0-3 wavetables 0-3 start
+ * 0x806: VOLUME
+ *bits 2-4 = 3 bit internal ASC volume,
+ *bits 5-7 = volume control sent to Sony sound chip
+ * 0x807: CLOCK RATE
+ *0 = Mac 22257 Hz,
+ *1 = undefined,
+ *2 = 22050 Hz,
+ *3 = 44100 Hz
+ * 0x80a: PLAY REC A
+ * 0x80f: TEST
+ *bits 6-7 = digital test,
+ *bits 4-5 = analog test
+ * 0x810: WAVETABLE 0 PHASE
+ *big-endian 9.15 fixed-point, only 24 bits valid
+ * 0x814: WAVETABLE 0 INCREMENT
+ *big-endian 9.15 fixed-point, only 24 bits valid
+ * 0x818: WAVETABLE 1 PHASE
+ * 0x81C: WAVETABLE 1 INCREMENT
+ * 0x820: WAVETABLE 2 PHASE
+ * 0x824: WAVETABLE 2 INCREMENT
+ * 0x828: WAVETABLE 3 PHASE
+ * 0x82C: WAVETABLE 3 INCREMENT
+ * 0x830: UNKNOWN START
+ *NetBSD writes Wavetable data here (are there more
+ *wavetables/channels than we know about?)
+ * 0x857: UNKNOWN END
+ */
+
+#define ASC_SIZE 0x2000
+
+enum {
+ASC_VERSION = 0x00,
+ASC_MODE= 0x01,
+ASC_CONTROL = 0x02,
+ASC_FIFOMODE= 0x03,
+ASC_FIFOIRQ = 0x04,
+ASC_WAVECTRL= 0x05,
+ASC_VOLUME = 0x06,
+ASC_CLOCK = 0x07,
+ASC_PLAYRECA= 0x0a,
+ASC_TEST= 0x0f,
+ASC_WAVETABLE = 0x10
+};
+
+#define ASC_FIFO_STATUS_HALF_FULL 1
+#define ASC
[PATCH 05/30] q800: move ROM memory region to Q800MachineState
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 13 +
include/hw/m68k/q800.h | 1 +
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index ee6175ceb4..6a000ceb75 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -371,7 +371,6 @@ static void q800_machine_init(MachineState *machine)
int bios_size;
ram_addr_t initrd_base;
int32_t initrd_size;
-MemoryRegion *rom;
MemoryRegion *io;
MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1);
uint8_t *prom;
@@ -643,11 +642,10 @@ static void q800_machine_init(MachineState *machine)
BOOTINFO1(param_ptr, BI_MAC_VROW, macfb_mode->stride);
BOOTINFO1(param_ptr, BI_MAC_SCCBASE, SCC_BASE);
-rom = g_malloc(sizeof(*rom));
-memory_region_init_ram_ptr(rom, NULL, "m68k_fake_mac.rom",
+memory_region_init_ram_ptr(&m->rom, NULL, "m68k_fake_mac.rom",
sizeof(fake_mac_rom), fake_mac_rom);
-memory_region_set_readonly(rom, true);
-memory_region_add_subregion(get_system_memory(), MACROM_ADDR, rom);
+memory_region_set_readonly(&m->rom, true);
+memory_region_add_subregion(get_system_memory(), MACROM_ADDR, &m->rom);
if (kernel_cmdline) {
BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
@@ -689,11 +687,10 @@ static void q800_machine_init(MachineState *machine)
} else {
uint8_t *ptr;
/* allocate and load BIOS */
-rom = g_malloc(sizeof(*rom));
-memory_region_init_rom(rom, NULL, "m68k_mac.rom", MACROM_SIZE,
+memory_region_init_rom(&m->rom, NULL, "m68k_mac.rom", MACROM_SIZE,
&error_abort);
filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-memory_region_add_subregion(get_system_memory(), MACROM_ADDR, rom);
+memory_region_add_subregion(get_system_memory(), MACROM_ADDR, &m->rom);
/* Load MacROM binary */
if (filename) {
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 5867c3ae33..2f3c720b8d 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -31,6 +31,7 @@ struct Q800MachineState {
MachineState parent_obj;
M68kCPU *cpu;
+MemoryRegion rom;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
--
2.30.2
[PATCH 01/30] q800: fix up minor spacing issues in hw_compat_q800 GlobalProperty array
Ensure there is a space before the final closing brace for all global
properties.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index b35ecafbc7..1aead224e2 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -719,14 +719,14 @@ static void q800_init(MachineState *machine)
}
static GlobalProperty hw_compat_q800[] = {
-{ "scsi-hd", "quirk_mode_page_vendor_specific_apple", "on"},
+{ "scsi-hd", "quirk_mode_page_vendor_specific_apple", "on" },
{ "scsi-hd", "vendor", " SEAGATE" },
{ "scsi-hd", "product", " ST225N" },
{ "scsi-hd", "ver", "1.0 " },
-{ "scsi-cd", "quirk_mode_page_apple_vendor", "on"},
-{ "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
-{ "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on"},
-{ "scsi-cd", "quirk_mode_page_truncated", "on"},
+{ "scsi-cd", "quirk_mode_page_apple_vendor", "on" },
+{ "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on" },
+{ "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on" },
+{ "scsi-cd", "quirk_mode_page_truncated", "on" },
{ "scsi-cd", "vendor", "MATSHITA" },
{ "scsi-cd", "product", "CD-ROM CR-8005" },
{ "scsi-cd", "ver", "1.0k" },
--
2.30.2
[PATCH 07/30] q800: introduce mac-io container memory region
Move all devices from the IO region to within the container in preparation
for updating the IO aliasing mechanism.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 6 ++
include/hw/m68k/q800.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index c22a98d616..6399631ed0 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -410,6 +410,12 @@ static void q800_machine_init(MachineState *machine)
/* RAM */
memory_region_add_subregion(get_system_memory(), 0, machine->ram);
+/*
+ * Create container for all IO devices
+ */
+memory_region_init(&m->macio, NULL, "mac-io", IO_SLICE);
+memory_region_add_subregion(get_system_memory(), IO_BASE, &m->macio);
+
/*
* Memory from IO_BASE to IO_BASE + IO_SLICE is repeated
* from IO_BASE + IO_SLICE to IO_BASE + IO_SIZE
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index de02af53be..156872a124 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -33,6 +33,7 @@ struct Q800MachineState {
M68kCPU *cpu;
MemoryRegion rom;
DeviceState *glue;
+MemoryRegion macio;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
--
2.30.2
[PATCH 11/30] q800: implement additional machine id bits on VIA1 port A
Co-developed-by: Laurent Vivier
Signed-off-by: Mark Cave-Ayland
---
hw/misc/mac_via.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
index 076d18e5fd..f90a22a067 100644
--- a/hw/misc/mac_via.c
+++ b/hw/misc/mac_via.c
@@ -114,6 +114,9 @@
#define VIA1A_CPUID10x04/* CPU id bit 0 on RBV, others */
#define VIA1A_CPUID20x10/* CPU id bit 0 on RBV, others */
#define VIA1A_CPUID30x40/* CPU id bit 0 on RBV, others */
+#define VIA1A_CPUID_MASK (VIA1A_CPUID0 | VIA1A_CPUID1 | \
+ VIA1A_CPUID2 | VIA1A_CPUID3)
+#define VIA1A_CPUID_Q800 (VIA1A_CPUID0 | VIA1A_CPUID2)
/*
* Info on VIA1B is from Macintosh Family Hardware & MkLinux.
@@ -871,9 +874,18 @@ static uint64_t mos6522_q800_via1_read(void *opaque,
hwaddr addr, unsigned size)
{
MOS6522Q800VIA1State *s = MOS6522_Q800_VIA1(opaque);
MOS6522State *ms = MOS6522(s);
+uint64_t ret;
addr = (addr >> 9) & 0xf;
-return mos6522_read(ms, addr, size);
+ret = mos6522_read(ms, addr, size);
+switch (addr) {
+case VIA_REG_A:
+case VIA_REG_ANH:
+/* Quadra 800 Id */
+ret = (ret & ~VIA1A_CPUID_MASK) | VIA1A_CPUID_Q800;
+break;
+}
+return ret;
}
static void mos6522_q800_via1_write(void *opaque, hwaddr addr, uint64_t val,
--
2.30.2
[PATCH 10/30] q800: add machine id register
MacOS reads this address to identify the hardware.
This is a basic implementation returning the ID of Quadra 800.
Details:
http://mess.redump.net/mess/driver_info/mac_technical_notes
"There are 3 ID schemes [...]
The third and most scalable is a machine ID register at 0x5ffc.
The top word must be 0xa55a to be valid. Then bits 15-11 are 0 for
consumer Macs, 1 for portables, 2 for high-end 68k, and 3 for high-end
PowerPC. Bit 10 is 1 if additional ID bits appear elsewhere (e.g. in VIA1).
The rest of the bits are a per-model identifier.
Model Lower 16 bits of ID
...
Quadra/Centris 610/650/800 0x2BAD"
Co-developed-by: Laurent Vivier
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 29 +
include/hw/m68k/q800.h | 1 +
2 files changed, 30 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 456407898e..c1d4b98cc0 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -91,6 +91,9 @@
#define Q800_NUBUS_SLOTS_AVAILABLE(BIT(0x9) | BIT(0xc) | BIT(0xd) | \
BIT(0xe))
+/* Quadra 800 machine ID */
+#define Q800_MACHINE_ID0xa55a2bad
+
/*
* The GLUE (General Logic Unit) is an Apple custom integrated circuit chip
* that performs a variety of functions (RAM management, clock generation,
...).
@@ -426,6 +429,27 @@ static const MemoryRegionOps macio_alias_ops = {
},
};
+static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
+{
+return Q800_MACHINE_ID;
+}
+
+static void machine_id_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size)
+{
+return;
+}
+
+static const MemoryRegionOps machine_id_ops = {
+.read = machine_id_read,
+.write = machine_id_write,
+.endianness = DEVICE_BIG_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+};
+
static void q800_machine_init(MachineState *machine)
{
Q800MachineState *m = Q800_MACHINE(machine);
@@ -488,6 +512,11 @@ static void q800_machine_init(MachineState *machine)
memory_region_add_subregion(get_system_memory(), IO_BASE + IO_SLICE,
&m->macio_alias);
+memory_region_init_io(&m->machine_id, NULL, &machine_id_ops, NULL,
+ "Machine ID", 4);
+memory_region_add_subregion(get_system_memory(), 0x5ffc,
+&m->machine_id);
+
/* IRQ Glue */
m->glue = qdev_new(TYPE_GLUE);
object_property_set_link(OBJECT(m->glue), "cpu", OBJECT(m->cpu),
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index d0e37cc665..e57aec849a 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -37,6 +37,7 @@ struct Q800MachineState {
MemoryRegion macio;
MemoryRegion macio_alias;
+MemoryRegion machine_id;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
--
2.30.2
[PATCH 02/30] q800: introduce Q800MachineState
This provides an overall container and owner for Machine-related objects such
as MemoryRegions.
Signed-off-by: Mark Cave-Ayland
---
MAINTAINERS| 1 +
hw/m68k/q800.c | 2 ++
include/hw/m68k/q800.h | 37 +
3 files changed, 40 insertions(+)
create mode 100644 include/hw/m68k/q800.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c93ab0ee5..86a1b88863 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1234,6 +1234,7 @@ F: include/hw/misc/mac_via.h
F: include/hw/nubus/*
F: include/hw/display/macfb.h
F: include/hw/block/swim.h
+F: include/hw/m68k/q800.h
virt
M: Laurent Vivier
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 1aead224e2..bdccd93c7f 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -38,6 +38,7 @@
#include "standard-headers/asm-m68k/bootinfo.h"
#include "standard-headers/asm-m68k/bootinfo-mac.h"
#include "bootinfo.h"
+#include "hw/m68k/q800.h"
#include "hw/misc/mac_via.h"
#include "hw/input/adb.h"
#include "hw/nubus/mac-nubus-bridge.h"
@@ -748,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void
*data)
static const TypeInfo q800_machine_typeinfo = {
.name = MACHINE_TYPE_NAME("q800"),
.parent = TYPE_MACHINE,
+.instance_size = sizeof(Q800MachineState),
.class_init = q800_machine_class_init,
};
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
new file mode 100644
index 00..560fd6f93d
--- /dev/null
+++ b/include/hw/m68k/q800.h
@@ -0,0 +1,37 @@
+/*
+ * QEMU Motorla 680x0 Macintosh hardware System Emulator
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_Q800_H
+#define HW_Q800_H
+
+/*
+ * The main Q800 machine
+ */
+
+struct Q800MachineState {
+MachineState parent_obj;
+};
+
+#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
+OBJECT_DECLARE_SIMPLE_TYPE(Q800MachineState, q800, Q800_MACHINE, MachineState)
+
+#endif
--
2.30.2
[PATCH 00/30] q800: add support for booting MacOS Classic
This series contains the remaining patches needed to allow QEMU's q800 machine to boot MacOS Classic when used in conjunction with a real Quadra 800 ROM image. In fact with this series applied it is possible to boot all of the following OSs: - MacOS 7.1 - 8.1, with or without virtual memory enabled - A/UX 3.0.1 - NetBSD 9.3 - Linux (via EMILE) If you are ready to experience some 90s nostalgia then all you need is to grab yourself a copy of the Quadra 800 ROM (checksum 0xf1acad13) and a suitable install ISO as follows: # Prepare a PRAM image $ qemu-img create -f raw pram.img 256b # Launch QEMU with blank disk and install CDROM $ ./qemu-system-m68k \ -M q800 \ -m 128 \ -bios Quadra800.rom \ -drive file=pram.img,format=raw,if=mtd \ -drive file=disk.img,media=disk,format=raw,if=none,id=hd \ -device scsi-hd,scsi-id=0,drive=hd \ -drive file=cdrom.iso,media=cdrom,if=none,id=cd \ -device scsi-cd,scsi-id=3,drive=cd And off you go! For more in-depth information about the installation process I highly recommend the installation guide over at emaculation.com [1]. Compatibility is generally very good, and I'm pleased to report it is possible to run one of the most popular productivity apps from the 90s [2]. I'd like to add a big thank you to all the people who have helped me work on this series, including testing on real hardware, answering questions about MacOS Classic internals and helping to diagnose and fix bugs in the 68k emulation. In particular thanks go to Laurent Vivier, Finn Thain, Howard Spoelstra, Volker Rümelin, Richard Henderson, Martin Husemann, Rin Okuyama, Elliot Nunn, and SolraBizna. Signed-off-by: Mark Cave-Ayland [1] https://www.emaculation.com/doku.php/qemu [2] https://www.youtube.com/watch?v=yI21gURQ1Ew Mark Cave-Ayland (30): q800: fix up minor spacing issues in hw_compat_q800 GlobalProperty array q800: introduce Q800MachineState q800: rename q800_init() to q800_machine_init() q800: move CPU object into Q800MachineState q800: move ROM memory region to Q800MachineState q800: move GLUE device to Q800MachineState q800: introduce mac-io container memory region q800: reimplement mac-io region aliasing using IO memory region q800: add djMEMC memory controller q800: add machine id register q800: implement additional machine id bits on VIA1 port A q800: add IOSB subsystem q800: allow accesses to RAM area even if less memory is available audio: add Apple Sound Chip (ASC) emulation asc: generate silence if FIFO empty but engine still running q800: add Apple Sound Chip (ASC) audio to machine q800: add easc bool machine class property to switch between ASC and EASC swim: add trace events for IWM and ISM registers swim: split into separate IWM and ISM register blocks swim: update IWM/ISM register block decoding mac_via: work around underflow in TimeDBRA timing loop in SETUPTIMEK mac_via: fix rtc command decoding from PRAM addresses 0x0 to 0xf mac_via: fix rtc command decoding for the PRAM seconds registers mac_via: workaround NetBSD ADB bus enumeration issue mac_via: implement ADB_STATE_IDLE state if shift register in input mode mac_via: always clear ADB interrupt when switching to A/UX mode q800: add ESCC alias at 0xc000 q800: add alias for MacOS toolbox ROM at 0x4000 mac_via: extend timer calibration hack to work with A/UX mac_via: work around QEMU unaligned MMIO access bug MAINTAINERS | 7 + hw/audio/Kconfig | 3 + hw/audio/asc.c| 691 ++ hw/audio/meson.build | 1 + hw/audio/trace-events | 10 + hw/block/swim.c | 261 +- hw/block/trace-events | 8 + hw/m68k/Kconfig | 3 + hw/m68k/q800.c| 312 ++--- hw/misc/Kconfig | 6 + hw/misc/djmemc.c | 154 + hw/misc/iosb.c| 156 + hw/misc/mac_via.c | 288 +++- hw/misc/meson.build | 2 + hw/misc/trace-events | 11 + include/hw/audio/asc.h| 75 + include/hw/block/swim.h | 21 +- include/hw/m68k/q800.h| 50 +++ include/hw/misc/djmemc.h | 46 +++ include/hw/misc/iosb.h| 41 +++ include/hw/misc/mac_via.h | 7 +- 21 files changed, 1993 insertions(+), 160 deletions(-) create mode 100644 hw/audio/asc.c create mode 100644 hw/misc/djmemc.c create mode 100644 hw/misc/iosb.c create mode 100644 include/hw/audio/asc.h create mode 100644 include/hw/m68k/q800.h create mode 100644 include/hw/misc/djmemc.h create mode 100644 include/hw/misc/iosb.h -- 2.30.2
[PATCH 04/30] q800: move CPU object into Q800MachineState
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 10 +-
include/hw/m68k/q800.h | 4 +++-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 976da06231..ee6175ceb4 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -363,7 +363,7 @@ static uint8_t fake_mac_rom[] = {
static void q800_machine_init(MachineState *machine)
{
-M68kCPU *cpu = NULL;
+Q800MachineState *m = Q800_MACHINE(machine);
int linux_boot;
int32_t kernel_size;
uint64_t elf_entry;
@@ -406,8 +406,8 @@ static void q800_machine_init(MachineState *machine)
}
/* init CPUs */
-cpu = M68K_CPU(cpu_create(machine->cpu_type));
-qemu_register_reset(main_cpu_reset, cpu);
+m->cpu = M68K_CPU(cpu_create(machine->cpu_type));
+qemu_register_reset(main_cpu_reset, m->cpu);
/* RAM */
memory_region_add_subregion(get_system_memory(), 0, machine->ram);
@@ -429,7 +429,7 @@ static void q800_machine_init(MachineState *machine)
/* IRQ Glue */
glue = qdev_new(TYPE_GLUE);
-object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
+object_property_set_link(OBJECT(glue), "cpu", OBJECT(m->cpu),
&error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
/* VIA 1 */
@@ -604,7 +604,7 @@ static void q800_machine_init(MachineState *machine)
macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
-cs = CPU(cpu);
+cs = CPU(m->cpu);
if (linux_boot) {
uint64_t high;
void *param_blob, *param_ptr, *param_rng_seed;
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 560fd6f93d..5867c3ae33 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -29,9 +29,11 @@
struct Q800MachineState {
MachineState parent_obj;
+
+M68kCPU *cpu;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
-OBJECT_DECLARE_SIMPLE_TYPE(Q800MachineState, q800, Q800_MACHINE, MachineState)
+OBJECT_DECLARE_SIMPLE_TYPE(Q800MachineState, Q800_MACHINE)
#endif
--
2.30.2
Re: [RFC PATCH v1 0/9] Hypervisor-Enforced Kernel Integrity
On 5/5/2023 8:20 AM, Mickaël Salaün wrote: Hi, This patch series is a proof-of-concept that implements new KVM features (extended page tracking, MBEC support, CR pinning) and defines a new API to protect guest VMs. No VMM (e.g., Qemu) modification is required. The main idea being that kernel self-protection mechanisms should be delegated to a more privileged part of the system, hence the hypervisor. It is still the role of the guest kernel to request such restrictions according to its Only for the guest kernel images here? Why not for the host OS kernel? Embedded devices w/ Android you have mentioned below supports the host OS as well it seems, right? Do we suggest that all the functionalities should be implemented in the Hypervisor (NS-EL2 for ARM) or even at Secure EL like Secure-EL1 (ARM). I am hoping that whatever we suggest the interface here from the Guest to the Hypervisor becomes the ABI right? # Current limitations The main limitation of this patch series is the statically enforced permissions. This is not an issue for kernels without module but this needs to be addressed. Mechanisms that dynamically impact kernel executable memory are not handled for now (e.g., kernel modules, tracepoints, eBPF JIT), and such code will need to be authenticated. Because the hypervisor is highly privileged and critical to the security of all the VMs, we don't want to implement a code authentication mechanism in the hypervisor itself but delegate this verification to something much less privileged. We are thinking of two ways to solve this: implement this verification in the VMM or spawn a dedicated special VM (similar to Windows's VBS). There are pros on cons to each approach: complexity, verification code ownership (guest's or VMM's), access to guest memory (i.e., confidential computing). Do you foresee the performance regressions due to lot of tracking here? Production kernels do have lot of tracepoints and we use it as feature in the GKI kernel for the vendor hooks implementation and in those cases every vendor driver is a module. Separate VM further fragments this design and delegates more of it to proprietary solutions? Do you have any performance numbers w/ current RFC? ---Trilok Soni
Re: [PATCH v1 2/9] KVM: x86/mmu: Add support for prewrite page tracking
On 5/5/23 12:31, Sean Christopherson wrote: > On Fri, May 05, 2023, Micka�l Sala�n wrote: >> >> On 05/05/2023 18:28, Sean Christopherson wrote: >>> I have no doubt that we'll need to solve performance and scaling issues >>> with the >>> memory attributes implementation, e.g. to utilize xarray multi-range support >>> instead of storing information on a per-4KiB-page basis, but AFAICT, the >>> core >>> idea is sound. And a very big positive from a maintenance perspective is >>> that >>> any optimizations, fixes, etc. for one use case (CoCo vs. hardening) should >>> also >>> benefit the other use case. >>> >>> [1] https://lore.kernel.org/all/20230311002258.852397-22-sea...@google.com >>> [2] https://lore.kernel.org/all/y2wb48kd0j4vg...@google.com >>> [3] https://lore.kernel.org/all/y1a1i9vbj%2fpvm...@google.com >> >> I agree, I used this mechanism because it was easier at first to rely on a >> previous work, but while I was working on the MBEC support, I realized that >> it's not the optimal way to do it. >> >> I was thinking about using a new special EPT bit similar to >> EPT_SPTE_HOST_WRITABLE, but it may not be portable though. What do you >> think? > > On x86, SPTEs are even more ephemeral than memslots. E.g. for historical > reasons, > KVM zaps all SPTEs if _any_ memslot is deleted, which is problematic if the > guest > is moving around BARs, using option ROMs, etc. > > ARM's pKVM tracks metadata in its stage-2 PTEs, i.e. doesn't need an xarray to > otrack attributes, but that works only because pKVM is more privileged than > the > host kernel, and the shared vs. private memory attribute that pKVM cares about > is very, very restricted in how it can be used and changed. > > I tried shoehorning private vs. shared metadata into x86's SPTEs in the past, > and > it ended up being a constant battle with the kernel, e.g. page migration, and > with > KVM itself, e.g. the above memslot mess. Sorry for the delay in responding to this. I wanted to study the KVM code and fully understand your comment before responding. Yes, I quite agree with you. I will make an attempt to address this in the next version. I am working on it right now. Thanks. Madhavan
Re: [PATCH v7 08/14] KVM: Rename mmu_notifier_*
On Wed, May 24, 2023 at 01:16:03PM -0700, Sean Christopherson wrote:
> Atomics aren't memory barriers on all architectures, e.g. see the various
> definitions of smp_mb__after_atomic().
>
> Even if atomic operations did provide barriers, using an atomic would be
> overkill
> and a net negative. On strongly ordered architectures like x86, memory
> barriers are
> just compiler barriers, whereas atomics may be more expensive.
Not quite, smp_{r,w}mb() and smp_mb__{before,after}_atomic() are
compiler barriers on the TSO archs, but smp_mb() very much isn't. TSO
still allows stores to be delayed vs later loads (iow it doesn't pretend
to hide the store buffer).
> Of course, the only
> accesses outside of mmu_lock are reads, so on x86 that "atomic" access is
> just a
> READ_ONCE() load, but that's not the case for all architectures.
This is true on *all* archs. atomic_set() and atomic_read() are no more
and no less than WRITE_ONCE() / READ_ONCE().
> Anyways, the point is that atomics and memory barriers are different things
> that
> serve different purposes.
This is true; esp. on the weakly ordered architectures where atomics do
not naturally imply any ordering.
Re: [PATCH v7 08/14] KVM: Rename mmu_notifier_*
On Wed, May 24, 2023 at 11:42:15AM +0530, Kautuk Consul wrote: > My comment was based on the assumption that "all atomic operations are > implicit memory barriers". If that assumption is true then we won't need It is not -- also see Documentation/atomic_t.txt. Specifically atomic_read() doesn't imply any ordering on any architecture including the strongly ordered TSO-archs (like x86).
Re: [PATCH v7 08/14] KVM: Rename mmu_notifier_*
On Wed, May 24, 2023, Kautuk Consul wrote:
> On 2023-05-23 07:19:43, Sean Christopherson wrote:
> > On Tue, May 23, 2023, Kautuk Consul wrote:
> > > On 2022-07-06 16:20:10, Chao Peng wrote:
> > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > > index e9153b54e2a4..c262ebb168a7 100644
> > > > --- a/include/linux/kvm_host.h
> > > > +++ b/include/linux/kvm_host.h
> > > > @@ -765,10 +765,10 @@ struct kvm {
> > > >
> > > > #if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)
> > > > struct mmu_notifier mmu_notifier;
> > > > - unsigned long mmu_notifier_seq;
> > > > - long mmu_notifier_count;
> > > > - gfn_t mmu_notifier_range_start;
> > > > - gfn_t mmu_notifier_range_end;
> > > > + unsigned long mmu_updating_seq;
> > > > + long mmu_updating_count;
> > >
> > > Can we convert mmu_updating_seq and mmu_updating_count to atomic_t ?
> >
> > Heh, can we? Yes. Should we? No.
> >
> > > I see that not all accesses to these are under the kvm->mmu_lock
> > > spinlock.
> >
> > Ya, working as intended. Ignoring gfn_to_pfn_cache for the moment, all
> > accesses
> > to mmu_invalidate_in_progress (was mmu_notifier_count / mmu_updating_count
> > above)
> > are done under mmu_lock. And for for mmu_notifier_seq (mmu_updating_seq
> > above),
> > all writes and some reads are done under mmu_lock. The only reads that are
> > done
> > outside of mmu_lock are the initial snapshots of the sequence number.
> >
> > gfn_to_pfn_cache uses a different locking scheme, the comments in
> > mmu_notifier_retry_cache() do a good job explaining the ordering.
> >
> > > This will also remove the need for putting separate smp_wmb() and
> > > smp_rmb() memory barriers while accessing these structure members.
> >
> > No, the memory barriers aren't there to provide any kind of atomicity. The
> > barriers
> > exist to ensure that stores and loads to/from the sequence and invalidate
> > in-progress
> > counts are ordered relative to the invalidation (stores to counts) and
> > creation (loads)
> > of SPTEs. Making the counts atomic changes nothing because atomic
> > operations don't
> > guarantee the necessary ordering.
> I'm not saying that the memory barriers provide atomicity.
> My comment was based on the assumption that "all atomic operations are
> implicit memory barriers". If that assumption is true then we won't need
> the memory barriers here if we use atomic operations for protecting
> these 2 structure members.
Atomics aren't memory barriers on all architectures, e.g. see the various
definitions of smp_mb__after_atomic().
Even if atomic operations did provide barriers, using an atomic would be
overkill
and a net negative. On strongly ordered architectures like x86, memory
barriers are
just compiler barriers, whereas atomics may be more expensive. Of course, the
only
accesses outside of mmu_lock are reads, so on x86 that "atomic" access is just a
READ_ONCE() load, but that's not the case for all architectures.
Anyways, the point is that atomics and memory barriers are different things that
serve different purposes.
New container build error: mountpoint does not exit
Hi Eldon, New this morning are some odd failures in the container build stage, e.g https://gitlab.com/qemu-project/qemu/-/jobs/4345796216#L235 cgroups: cgroup mountpoint does not exist: unknown There are several such failures in that pipeline. I've not seen this before, nor was it happening yesterday. Any ideas? r~
[PATCH] tcg: Fix register move type in tcg_out_ld_helper_ret
The first move was incorrectly using TCG_TYPE_I32 while the second
move was correctly using TCG_TYPE_REG. This prevents a 64-bit host
from moving all 128-bits of the return value.
Fixes: ebebea53ef8 ("tcg: Support TCG_TYPE_I128 in
tcg_out_{ld,st}_helper_{args,ret}")
Signed-off-by: Richard Henderson
---
tcg/tcg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index ac30d484f5..2352ca4ade 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -5736,8 +5736,8 @@ static void tcg_out_ld_helper_ret(TCGContext *s, const
TCGLabelQemuLdst *ldst,
mov[0].dst = ldst->datalo_reg;
mov[0].src =
tcg_target_call_oarg_reg(TCG_CALL_RET_NORMAL, HOST_BIG_ENDIAN);
-mov[0].dst_type = TCG_TYPE_I32;
-mov[0].src_type = TCG_TYPE_I32;
+mov[0].dst_type = TCG_TYPE_REG;
+mov[0].src_type = TCG_TYPE_REG;
mov[0].src_ext = TCG_TARGET_REG_BITS == 32 ? MO_32 : MO_64;
mov[1].dst = ldst->datahi_reg;
--
2.34.1
Re: [PATCH v3 2/7] migration: Implement switchover ack logic
On Sun, May 21, 2023 at 06:18:03PM +0300, Avihai Horon wrote:
> Implement switchover ack logic. This prevents the source from stopping
> the VM and completing the migration until an ACK is received from the
> destination that it's OK to do so.
>
> To achieve this, a new SaveVMHandlers handler switchover_ack_needed()
> and a new return path message MIG_RP_MSG_SWITCHOVER_ACK are added.
>
> The switchover_ack_needed() handler is called during migration setup
> both in the source and the destination to check if switchover ack is
> used by the migrated device.
>
> When switchover is approved by all migrated devices in the destination
> that support this capability, the MIG_RP_MSG_INITIAL_DATA_LOADED_ACK
s/MIG_RP_MSG_INITIAL_DATA_LOADED_ACK/MIG_RP_MSG_SWITCHOVER_ACK/
> return path message is sent to the source to notify it that it's OK to
> do switchover.
>
> Signed-off-by: Avihai Horon
> ---
> include/migration/register.h | 3 ++
> migration/migration.h| 16 +++
> migration/savevm.h | 2 ++
> migration/migration.c| 42 +--
> migration/savevm.c | 56
> migration/trace-events | 4 +++
> 6 files changed, 121 insertions(+), 2 deletions(-)
>
> diff --git a/include/migration/register.h b/include/migration/register.h
> index a8dfd8fefd..cda36d377b 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -71,6 +71,9 @@ typedef struct SaveVMHandlers {
> int (*load_cleanup)(void *opaque);
> /* Called when postcopy migration wants to resume from failure */
> int (*resume_prepare)(MigrationState *s, void *opaque);
> +
> +/* Checks if switchover ack should be used. Called both in src and dest
> */
> +bool (*switchover_ack_needed)(void *opaque);
Sorry if I'm going to suggest something that overwrites what I
suggested.. :( Luckily, not so much.
When I read the new series I just noticed maybe it's still better to only
use this on dst, and always do the ACK.
The problem is based on your patch 1 description, the RP ACK message will
be sent if the switchover-ack cap is set, but actually it's conditionally
in the current impl just to handle num==0 case, so either the impl needs
change or the desc needs change.
It turns out it'll be even cleaner to always send it. If so..
> } SaveVMHandlers;
>
> int register_savevm_live(const char *idstr,
> diff --git a/migration/migration.h b/migration/migration.h
> index 48a46123a0..e209860cce 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -209,6 +209,13 @@ struct MigrationIncomingState {
> * contains valid information.
> */
> QemuMutex page_request_mutex;
> +
> +/*
> + * Number of devices that have yet to approve switchover. When this
> reaches
> + * zero an ACK that it's OK to do switchover is sent to the source. No
> lock
> + * is needed as this field is updated serially.
> + */
> +unsigned int switchover_ack_pending_num;
> };
>
> MigrationIncomingState *migration_incoming_get_current(void);
> @@ -437,6 +444,14 @@ struct MigrationState {
>
> /* QEMU_VM_VMDESCRIPTION content filled for all non-iterable devices. */
> JSONWriter *vmdesc;
> +
> +/* Number of devices that use switchover ack capability */
> +unsigned int switchover_ack_user_num;
... we save this field, then...
> +/*
> + * Indicates whether an ACK from the destination that it's OK to do
> + * switchover has been received.
> + */
> +bool switchover_acked;
> };
>
> void migrate_set_state(int *state, int old_state, int new_state);
> @@ -477,6 +492,7 @@ int
> migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
> void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis,
> char *block_name);
> void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
> +int migrate_send_rp_switchover_ack(MigrationIncomingState *mis);
>
> void dirty_bitmap_mig_before_vm_start(void);
> void dirty_bitmap_mig_cancel_outgoing(void);
> diff --git a/migration/savevm.h b/migration/savevm.h
> index fb636735f0..5c3e1a026b 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -32,6 +32,7 @@
> bool qemu_savevm_state_blocked(Error **errp);
> void qemu_savevm_non_migratable_list(strList **reasons);
> void qemu_savevm_state_setup(QEMUFile *f);
> +void qemu_savevm_state_switchover_ack_needed(MigrationState *ms);
> bool qemu_savevm_state_guest_unplug_pending(void);
> int qemu_savevm_state_resume_prepare(MigrationState *s);
> void qemu_savevm_state_header(QEMUFile *f);
> @@ -65,6 +66,7 @@ int qemu_loadvm_state(QEMUFile *f);
> void qemu_loadvm_state_cleanup(void);
> int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> int qemu_load_device_state(QEMUFile *f);
> +int qemu_loadvm_approve_switchover(void);
> int qemu_savevm_state_complete_precopy_non_iterable(QEMUFi
Re: [PATCH v3 1/7] migration: Add switchover ack capability
On Sun, May 21, 2023 at 06:18:02PM +0300, Avihai Horon wrote: > Migration downtime estimation is calculated based on bandwidth and > remaining migration data. This assumes that loading of migration data in > the destination takes a negligible amount of time and that downtime > depends only on network speed. > > While this may be true for RAM, it's not necessarily true for other > migration users. For example, loading the data of a VFIO device in the > destination might require from the device to allocate resources, prepare > internal data structures and so on. These operations can take a > significant amount of time which can increase migration downtime. > > This patch adds a new capability "switchover ack" that prevents the > source from stopping the VM and completing the migration until an ACK > is received from the destination that it's OK to do so. > > This can be used by migrated devices in various ways to reduce downtime. > For example, a device can send initial precopy metadata to pre-allocate > resources in the destination and use this capability to make sure that > the pre-allocation is completed before the source VM is stopped, so it > will have full effect. > > This new capability relies on the return path capability to communicate > from the destination back to the source. > > The actual implementation of the capability will be added in the > following patches. > > Signed-off-by: Avihai Horon Reviewed-by: Peter Xu -- Peter Xu
[PATCH] Prepare bcm properties for videocore 4
Hello!
Sorry for a quite a big patch, but most of the changes are the same type.
Most of the patch is about a definition of new constants/structs and replacing
magic numbers with those constants.
Signed-off-by: Sergey Kambalin
---
hw/misc/bcm2835_property.c| 314 +-
include/hw/arm/raspi_platform.h | 5 +
include/hw/misc/raspberrypi-fw-defs.h | 169 ++
3 files changed, 431 insertions(+), 57 deletions(-)
create mode 100644 include/hw/misc/raspberrypi-fw-defs.h
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
index 251b3d865d..70a8ed0a8c 100644
--- a/hw/misc/bcm2835_property.c
+++ b/hw/misc/bcm2835_property.c
@@ -12,10 +12,56 @@
#include "migration/vmstate.h"
#include "hw/irq.h"
#include "hw/misc/bcm2835_mbox_defs.h"
+#include "hw/misc/raspberrypi-fw-defs.h"
#include "sysemu/dma.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "trace.h"
+#include "hw/arm/raspi_platform.h"
+
+#define RPI_EXP_GPIO_BASE 128
+#define VC4_GPIO_EXPANDER_COUNT 8
+
+struct vc4_display_settings_t {
+uint32_t display_num;
+uint32_t width;
+uint32_t height;
+uint32_t depth;
+uint16_t pitch;
+uint32_t virtual_width;
+uint32_t virtual_height;
+uint16_t virtual_width_offset;
+uint32_t virtual_height_offset;
+unsigned long fb_bus_address;
+} QEMU_PACKED;
+
+enum rpi_firmware_clk_id {
+RPI_FIRMWARE_EMMC_CLK_ID = 1,
+RPI_FIRMWARE_UART_CLK_ID,
+RPI_FIRMWARE_ARM_CLK_ID,
+RPI_FIRMWARE_CORE_CLK_ID,
+RPI_FIRMWARE_V3D_CLK_ID,
+RPI_FIRMWARE_H264_CLK_ID,
+RPI_FIRMWARE_ISP_CLK_ID,
+RPI_FIRMWARE_SDRAM_CLK_ID,
+RPI_FIRMWARE_PIXEL_CLK_ID,
+RPI_FIRMWARE_PWM_CLK_ID,
+RPI_FIRMWARE_HEVC_CLK_ID,
+RPI_FIRMWARE_EMMC2_CLK_ID,
+RPI_FIRMWARE_M2MC_CLK_ID,
+RPI_FIRMWARE_PIXEL_BVB_CLK_ID,
+RPI_FIRMWARE_VEC_CLK_ID,
+RPI_FIRMWARE_NUM_CLK_ID,
+};
+
+struct vc4_gpio_expander_t {
+uint32_t direction;
+uint32_t polarity;
+uint32_t term_en;
+uint32_t term_pull_up;
+uint32_t state;
+} vc4_gpio_expander[VC4_GPIO_EXPANDER_COUNT];
+
/* https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface */
@@ -28,6 +74,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState
*s, uint32_t value)
uint32_t tmp;
int n;
uint32_t offset, length, color;
+uint32_t gpio_num;
/*
* Copy the current state of the framebuffer config; we will update
@@ -51,48 +98,48 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState
*s, uint32_t value)
/* @(value + 8) : Request/response indicator */
resplen = 0;
switch (tag) {
-case 0x: /* End tag */
+case RPI_FWREQ_PROPERTY_END: /* End tag */
break;
-case 0x0001: /* Get firmware revision */
+case RPI_FWREQ_GET_FIRMWARE_REVISION: /* Get firmware revision */
stl_le_phys(&s->dma_as, value + 12, 346337);
resplen = 4;
break;
-case 0x00010001: /* Get board model */
+case RPI_FWREQ_GET_BOARD_MODEL: /* Get board model */
qemu_log_mask(LOG_UNIMP,
"bcm2835_property: 0x%08x get board model NYI\n",
tag);
resplen = 4;
break;
-case 0x00010002: /* Get board revision */
+case RPI_FWREQ_GET_BOARD_REVISION: /* Get board revision */
stl_le_phys(&s->dma_as, value + 12, s->board_rev);
resplen = 4;
break;
-case 0x00010003: /* Get board MAC address */
+case RPI_FWREQ_GET_BOARD_MAC_ADDRESS: /* Get board MAC address */
resplen = sizeof(s->macaddr.a);
dma_memory_write(&s->dma_as, value + 12, s->macaddr.a, resplen,
MEMTXATTRS_UNSPECIFIED);
break;
-case 0x00010004: /* Get board serial */
+case RPI_FWREQ_GET_BOARD_SERIAL: /* Get board serial */
qemu_log_mask(LOG_UNIMP,
"bcm2835_property: 0x%08x get board serial NYI\n",
tag);
resplen = 8;
break;
-case 0x00010005: /* Get ARM memory */
+case RPI_FWREQ_GET_ARM_MEMORY: /* Get ARM memory */
/* base */
stl_le_phys(&s->dma_as, value + 12, 0);
/* size */
stl_le_phys(&s->dma_as, value + 16, s->fbdev->vcram_base);
resplen = 8;
break;
-case 0x00010006: /* Get VC memory */
+case RPI_FWREQ_GET_VC_MEMORY: /* Get VC memory */
/* base */
stl_le_phys(&s->dma_as, value + 12, s->fbdev->vcram_base);
/* size */
stl_le_phys(&s->dma_as, value + 16, s->fbdev->vcram_size);
resplen = 8;
break;
-case 0x00028001: /* Set power state */
+case RPI_FWREQ_SET_POWER_STATE: /* Set power state */
/* Ass
Re: [PATCH 07/10] hw/arm/realview: Move 'mpcore_periphbase' to RealviewMachineClass
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: -int is_mpcore = 0; +bool is_mpcore = rmc->mpcore_periphbase != 0; bool is_pb = rmc->is_pb; uint32_t proc_id = 0; uint32_t sys_id; ram_addr_t low_ram_size; ram_addr_t ram_size = machine->ram_size; -hwaddr periphbase = 0; hwaddr periphbase = rmc->mpcore_periphbase; bool is_mpcore = periphbase != 0; would be cleaner and require fewer changes. Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH 09/10] hw/arm/realview: Use generic realview_common_machine_init()
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: The realview_board_type enum is now unused. Remove it and have all instances use the common realview_common_machine_init() method. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 46 ++ 1 file changed, 2 insertions(+), 44 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 08/10] hw/arm/realview: Move 'loader_start' to RealviewMachineClass
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: Instead of having each machine instance resolve its loader start address, set it once in their class_init() handler. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson
Re: [PATCH 10/10] hw/arm/realview: Set MachineClass::default_nic in machine_class_init()
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: Mark the default NIC via the new MachineClass->default_nic setting so that the machine-defaults code in vl.c can decide whether the default NIC is usable or not (for example when compiling with the "--without-default-devices" configure switch). Inspired-by: Thomas Huth Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 06/10] hw/arm/realview: Move 'is_pb' to RealviewMachineClass
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: Instead of having each machine instance set whether EP/PB, set it once in their class_init() handler. Arguably this could be extracted from the board_id field. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 05/10] hw/arm/realview: Move 'board_id' to RealviewMachineClass
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: Instead of having each machine instance resolve its board ID, set it once in their class_init() handler. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 04/10] hw/arm/realview: Factor realview_common_class_init() out
On 5/24/23 07:59, Philippe Mathieu-Daudé wrote: Introduce realview_common_class_init() where we'll set fields common to all Realview classes. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 03/10] hw/arm/realview: Introduce abstract RealviewMachineClass
On 5/24/23 07:58, Philippe Mathieu-Daudé wrote: Introduce the abstract QOM TYPE_REALVIEW_MACHINE to handle fields common to all Realview machines. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 02/10] hw/arm/realview: Declare QOM types using DEFINE_TYPES() macro
On 5/24/23 07:58, Philippe Mathieu-Daudé wrote: When multiple QOM types are registered in the same file, it is simpler to use the the DEFINE_TYPES() macro. Replace the type_init() / type_register_static() combination. Signed-off-by: Philippe Mathieu-Daudé --- hw/arm/realview.c | 50 ++- 1 file changed, 19 insertions(+), 31 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 01/10] hw/arm/realview: Simplify using 'break' statement
On 5/24/23 07:58, Philippe Mathieu-Daudé wrote:
The 'break' statement terminates the execution of the nearest
enclosing 'for' statement in which it appears.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/realview.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index a5aa2f046a..a52ff35084 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -88,7 +88,6 @@ static void realview_init(MachineState *machine,
I2CBus *i2c;
int n;
unsigned int smp_cpus = machine->smp.cpus;
-int done_nic = 0;
qemu_irq cpu_irq[4];
int is_mpcore = 0;
int is_pb = 0;
@@ -294,14 +293,13 @@ static void realview_init(MachineState *machine,
for(n = 0; n < nb_nics; n++) {
nd = &nd_table[n];
-if (!done_nic && (!nd->model ||
-strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0)) {
+if (!nd->model || strcmp(nd->model, is_pb ? "lan9118" : "smc91c111")
== 0) {
if (is_pb) {
lan9118_init(nd, 0x4e00, pic[28]);
} else {
smc91c111_init(nd, 0x4e00, pic[28]);
}
-done_nic = 1;
+break;
While I agree this preserves existing behaviour, it doesn't seem like the logic is
actually correct. This will only ever connect 1 of nb_nics.
r~
} else {
if (pci_bus) {
pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL);
[PATCH v3 1/2] meson: Split test for __int128_t type from __int128_t arithmetic
Older versions of clang have missing runtime functions for arithmetic
with -fsanitize=undefined (see 464e3671f9d5c), so we cannot use
__int128_t for implementing Int128. But __int128_t is present,
data movement works, and can be use for atomic128.
Probe for both CONFIG_INT128_TYPE and CONFIG_INT128, adjust
qemu/int128.h to define Int128Alias if CONFIG_INT128_TYPE,
and adjust the meson probe for atomics to use has_int128_type.
Signed-off-by: Richard Henderson
---
meson.build | 15 ++-
include/qemu/int128.h | 4 ++--
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/meson.build b/meson.build
index ef181ff2df..1f73c68a41 100644
--- a/meson.build
+++ b/meson.build
@@ -2536,7 +2536,13 @@ config_host_data.set('CONFIG_ATOMIC64', cc.links('''
return 0;
}'''))
-has_int128 = cc.links('''
+has_int128_type = cc.compiles('''
+ __int128_t a;
+ __uint128_t b;
+ int main(void) { b = a; }''')
+config_host_data.set('CONFIG_INT128_TYPE', has_int128_type)
+
+has_int128 = has_int128_type and cc.links('''
__int128_t a;
__uint128_t b;
int main (void) {
@@ -2545,10 +2551,9 @@ has_int128 = cc.links('''
a = a * a;
return 0;
}''')
-
config_host_data.set('CONFIG_INT128', has_int128)
-if has_int128
+if has_int128_type
# "do we have 128-bit atomics which are handled inline and specifically not
# via libatomic". The reason we can't use libatomic is documented in the
# comment starting "GCC is a house divided" in include/qemu/atomic128.h.
@@ -2557,7 +2562,7 @@ if has_int128
# __alignof(unsigned __int128) for the host.
atomic_test_128 = '''
int main(int ac, char **av) {
- unsigned __int128 *p = __builtin_assume_aligned(av[ac - 1], 16);
+ __uint128_t *p = __builtin_assume_aligned(av[ac - 1], 16);
p[1] = __atomic_load_n(&p[0], __ATOMIC_RELAXED);
__atomic_store_n(&p[2], p[3], __ATOMIC_RELAXED);
__atomic_compare_exchange_n(&p[4], &p[5], p[6], 0, __ATOMIC_RELAXED,
__ATOMIC_RELAXED);
@@ -2579,7 +2584,7 @@ if has_int128
config_host_data.set('CONFIG_CMPXCHG128', cc.links('''
int main(void)
{
- unsigned __int128 x = 0, y = 0;
+ __uint128_t x = 0, y = 0;
__sync_val_compare_and_swap_16(&x, y, x);
return 0;
}
diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index 9e46cfaefc..73624e8be7 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -481,7 +481,7 @@ static inline void bswap128s(Int128 *s)
* a possible structure and the native types. Ease parameter passing
* via use of the transparent union extension.
*/
-#ifdef CONFIG_INT128
+#ifdef CONFIG_INT128_TYPE
typedef union {
__uint128_t u;
__int128_t i;
@@ -489,6 +489,6 @@ typedef union {
} Int128Alias __attribute__((transparent_union));
#else
typedef Int128 Int128Alias;
-#endif /* CONFIG_INT128 */
+#endif /* CONFIG_INT128_TYPE */
#endif /* INT128_H */
--
2.34.1
[PATCH v3 2/2] qemu/atomic128: Add x86_64 atomic128-ldst.h
With CPUINFO_ATOMIC_VMOVDQA, we can perform proper atomic
load/store without cmpxchg16b.
Signed-off-by: Richard Henderson
---
host/include/x86_64/host/atomic128-ldst.h | 68 +++
1 file changed, 68 insertions(+)
create mode 100644 host/include/x86_64/host/atomic128-ldst.h
diff --git a/host/include/x86_64/host/atomic128-ldst.h
b/host/include/x86_64/host/atomic128-ldst.h
new file mode 100644
index 00..adc9332f91
--- /dev/null
+++ b/host/include/x86_64/host/atomic128-ldst.h
@@ -0,0 +1,68 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Load/store for 128-bit atomic operations, x86_64 version.
+ *
+ * Copyright (C) 2023 Linaro, Ltd.
+ *
+ * See docs/devel/atomics.rst for discussion about the guarantees each
+ * atomic primitive is meant to provide.
+ */
+
+#ifndef AARCH64_ATOMIC128_LDST_H
+#define AARCH64_ATOMIC128_LDST_H
+
+#ifdef CONFIG_INT128_TYPE
+#include "host/cpuinfo.h"
+#include "tcg/debug-assert.h"
+
+/*
+ * Through clang 16, with -mcx16, __atomic_load_n is incorrectly
+ * expanded to a read-write operation: lock cmpxchg16b.
+ */
+
+#define HAVE_ATOMIC128_RO likely(cpuinfo & CPUINFO_ATOMIC_VMOVDQA)
+#define HAVE_ATOMIC128_RW 1
+
+static inline Int128 atomic16_read_ro(const Int128 *ptr)
+{
+Int128Alias r;
+
+tcg_debug_assert(HAVE_ATOMIC128_RO);
+asm("vmovdqa %1, %0" : "=x" (r.i) : "m" (*ptr));
+
+return r.s;
+}
+
+static inline Int128 atomic16_read_rw(Int128 *ptr)
+{
+__int128_t *ptr_align = __builtin_assume_aligned(ptr, 16);
+Int128Alias r;
+
+if (HAVE_ATOMIC128_RO) {
+asm("vmovdqa %1, %0" : "=x" (r.i) : "m" (*ptr_align));
+} else {
+r.i = __sync_val_compare_and_swap_16(ptr_align, 0, 0);
+}
+return r.s;
+}
+
+static inline void atomic16_set(Int128 *ptr, Int128 val)
+{
+__int128_t *ptr_align = __builtin_assume_aligned(ptr, 16);
+Int128Alias new = { .s = val };
+
+if (HAVE_ATOMIC128_RO) {
+asm("vmovdqa %1, %0" : "=m"(*ptr_align) : "x" (new.i));
+} else {
+__int128_t old;
+do {
+old = *ptr_align;
+} while (!__sync_bool_compare_and_swap_16(ptr_align, old, new.i));
+}
+}
+#else
+/* Provide QEMU_ERROR stubs. */
+#include "host/include/generic/host/atomic128-ldst.h"
+#endif
+
+#endif /* AARCH64_ATOMIC128_LDST_H */
--
2.34.1
[PATCH v3 0/2] accel/tcg: Improvements to atomic128.h
Changes for v3: * Most of the v2 patch set merged, except x86_64 atomic128-ldst.h, which failed testing with clang-11 with debian 11. * New patch to change __int128_t detection. * This in turn enabled CONFIG_ATOMIC128, which was not ideal. This clang bug/mis-feature of using a cmpxchg sequence for implementing __atomic_load_n was already noted for aarch64, so I should have expected it would also be true for x86_64. Given that I am adding inline assembly for CPUINFO_ATOMIC_VMOVDQA anyway, this isn't a big deal, but I did need to adjust the ifdefs. r~ Richard Henderson (2): meson: Split test for __int128_t type from __int128_t arithmetic qemu/atomic128: Add x86_64 atomic128-ldst.h meson.build | 15 +++-- host/include/x86_64/host/atomic128-ldst.h | 68 +++ include/qemu/int128.h | 4 +- 3 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 host/include/x86_64/host/atomic128-ldst.h -- 2.34.1
Re: [PATCH] meson.build: Fix glib -Wno-unused-function workaround
On 24/5/23 19:31, Nicolas Saenz Julienne wrote:
We want to only enable '-Wno-unused-function' if glib's version is
smaller than '2.57.2' and has a G_DEFINE_AUTOPTR_CLEANUP_FUNC()
implementation that doesn't take into account unused functions. But the
compilation test isn't working as intended as '-Wunused-function' isn't
enabled while running it.
Let's enable it.
Fixes: fc9a809e0d28 ("build: move glib detection and workarounds to meson")
Signed-off-by: Nicolas Saenz Julienne
---
Meson logs before:
Running compile:
Working directory: /local/home/nsaenz/c/qemu/build/meson-private/tmp5fgb3xnk
Command line: clang -m64 -mcx16 -I/usr/include/glib-2.0
-I/usr/lib64/glib-2.0/include
/local/home/nsaenz/c/qemu/build/meson-private/tmp5fgb3xnk/testfile.c -o
/local/home/nsaenz/c/qemu/build/meson-private/tmp5fgb3xnk/output.obj -c
-D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration
-Werror=unknown-warning-option -Werror=unused-command-line-argument
-Werror=ignored-optimization-argument -std=gnu11 -Werror
Code:
#include
typedef struct Foo {
int i;
} Foo;
static void foo_free(Foo *f)
{
g_free(f);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
int main(void) { return 0; }
Compiler stdout:
Compiler stderr:
Meson logs after:
Running compile:
Working directory: /local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u
Command line: clang -m64 -mcx16 -I/usr/include/glib-2.0
-I/usr/lib64/glib-2.0/include
/local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u/testfile.c -o
/local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u/output.obj -c
-D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration
-Werror=unknown-warning-option -Werror=unused-command-line-argument
-Werror=ignored-optimization-argument -std=gnu11 -Wunused-function -Werror
Code:
#include
typedef struct Foo {
int i;
} Foo;
static void foo_free(Foo *f)
{
g_free(f);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
int main(void) { return 0; }
Compiler stdout:
Compiler stderr:
/local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u/testfile.c:10:3:
error: unused function 'glib_autoptr_cleanup_Foo' [-Werror,-Wunused-function]
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
^
/usr/include/glib-2.0/glib/gmacros.h:461:22: note: expanded from macro
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
static inline void _GLIB_AUTOPTR_FUNC_NAME(TypeName) (TypeName **_ptr) {
if (*_ptr) (func) (*_ptr); } \
^
/usr/include/glib-2.0/glib/gmacros.h:441:43: note: expanded from macro
'_GLIB_AUTOPTR_FUNC_NAME'
#define _GLIB_AUTOPTR_FUNC_NAME(TypeName) glib_autoptr_cleanup_##TypeName
^
:58:1: note: expanded from here
glib_autoptr_cleanup_Foo
^
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index ef181ff2df..71a4fcee52 100644
--- a/meson.build
+++ b/meson.build
@@ -780,7 +780,7 @@ if not cc.compiles('''
g_free(f);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
- int main(void) { return 0; }''', dependencies: glib_pc, args: ['-Werror'])
+ int main(void) { return 0; }''', dependencies: glib_pc, args:
['-Wunused-function', '-Werror'])
Oops, thanks!
Reviewed-by: Philippe Mathieu-Daudé
[PATCH] tests/decode: Emit TAP
We currently print FAIL for the failure of a succ_* test, but don't
return a failure exit code. Instead, convert the script to emit
Test Anything Protocol, which gives visibility into each subtest
as well as not relying on exit codes.
Suggested-by: Paolo Bonzini
Signed-off-by: Richard Henderson
---
tests/decode/check.sh | 36 ++--
tests/meson.build | 1 +
2 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/tests/decode/check.sh b/tests/decode/check.sh
index 95445a0115..a3d879a099 100755
--- a/tests/decode/check.sh
+++ b/tests/decode/check.sh
@@ -4,21 +4,37 @@
PYTHON=$1
DECODETREE=$2
-E=0
+E_FILES=`echo err_*.decode`
+S_FILES=`echo succ_*.decode`
-# All of these tests should produce errors
-for i in err_*.decode; do
+j=0
+for i in $E_FILES $S_FILES; do
+j=`expr $j + 1`
+done
+
+echo 1..$j
+
+j=0
+for i in $E_FILES; do
+j=`expr $j + 1`
+n=`basename $i .decode`
if $PYTHON $DECODETREE $i > /dev/null 2> /dev/null; then
-# Pass, aka failed to fail.
-echo FAIL: $i 1>&2
-E=1
+# Failed to fail.
+echo not ok $j $n
+else
+echo ok $j $n
fi
done
-for i in succ_*.decode; do
-if ! $PYTHON $DECODETREE $i > /dev/null 2> /dev/null; then
-echo FAIL:$i 1>&2
+for i in $S_FILES; do
+j=`expr $j + 1`
+n=`basename $i .decode`
+if $PYTHON $DECODETREE $i > /dev/null 2> /dev/null; then
+# Succeeded.
+echo ok $j $n
+else
+echo not ok $j $n
fi
done
-exit $E
+exit 0
diff --git a/tests/meson.build b/tests/meson.build
index 8e318ec513..137ef85ab6 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -77,6 +77,7 @@ endif
test('decodetree', sh,
args: [ files('decode/check.sh'), config_host['PYTHON'],
files('../scripts/decodetree.py') ],
workdir: meson.current_source_dir() / 'decode',
+ protocol: 'tap', verbose: true,
suite: 'decodetree')
if 'CONFIG_TCG' in config_all
--
2.34.1
Re: [PATCH v3 2/5] vdpa: add vhost_vdpa_reset_status_fd
On Wed, May 17, 2023 at 7:49 AM Jason Wang wrote:
>
> On Wed, May 17, 2023 at 1:46 PM Eugenio Perez Martin
> wrote:
> >
> > On Wed, May 17, 2023 at 5:14 AM Jason Wang wrote:
> > >
> > > On Tue, May 9, 2023 at 11:44 PM Eugenio Pérez wrote:
> > > >
> > > > This allows to reset a vhost-vdpa device from external subsystems like
> > > > vhost-net, since it does not have any struct vhost_dev by the time we
> > > > need to use it.
> > > >
> > > > It is used in subsequent patches to negotiate features
> > > > and probe for CVQ ASID isolation.
> > > >
> > > > Reviewed-by: Stefano Garzarella
> > > > Signed-off-by: Eugenio Pérez
> > > > ---
> > > > include/hw/virtio/vhost-vdpa.h | 1 +
> > > > hw/virtio/vhost-vdpa.c | 58 +++---
> > > > 2 files changed, 41 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/include/hw/virtio/vhost-vdpa.h
> > > > b/include/hw/virtio/vhost-vdpa.h
> > > > index c278a2a8de..28de7da91e 100644
> > > > --- a/include/hw/virtio/vhost-vdpa.h
> > > > +++ b/include/hw/virtio/vhost-vdpa.h
> > > > @@ -54,6 +54,7 @@ typedef struct vhost_vdpa {
> > > > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
> > > > } VhostVDPA;
> > > >
> > > > +void vhost_vdpa_reset_status_fd(int fd);
> > > > int vhost_vdpa_get_iova_range(int fd, struct vhost_vdpa_iova_range
> > > > *iova_range);
> > > >
> > > > int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, hwaddr
> > > > iova,
> > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > > > index bbabea18f3..7a2053b8d9 100644
> > > > --- a/hw/virtio/vhost-vdpa.c
> > > > +++ b/hw/virtio/vhost-vdpa.c
> > > > @@ -335,38 +335,45 @@ static const MemoryListener
> > > > vhost_vdpa_memory_listener = {
> > > > .region_del = vhost_vdpa_listener_region_del,
> > > > };
> > > >
> > > > -static int vhost_vdpa_call(struct vhost_dev *dev, unsigned long int
> > > > request,
> > > > - void *arg)
> > > > +static int vhost_vdpa_dev_fd(const struct vhost_dev *dev)
> > > > {
> > > > struct vhost_vdpa *v = dev->opaque;
> > > > -int fd = v->device_fd;
> > > > -int ret;
> > > >
> > > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
> > > > +return v->device_fd;
> > > > +}
> > >
> > > Nit: unless the vhost_dev structure is opaque to the upper layer, I
> > > don't see any advantage for having a dedicated indirect helper to get
> > > device_fd.
> > >
> >
> > The purpose was to not duplicate the assert, but sure it's not mandatory.
>
> Ok, but I think for new codes, we'd better avoid assert as much as possible.
>
I'll remove it, thanks!
> >
> > > > +
> > > > +static int vhost_vdpa_call_fd(int fd, unsigned long int request, void
> > > > *arg)
> > > > +{
> > > > +int ret = ioctl(fd, request, arg);
> > > >
> > > > -ret = ioctl(fd, request, arg);
> > > > return ret < 0 ? -errno : ret;
> > > > }
> > > >
> > > > -static int vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status)
> > > > +static int vhost_vdpa_call(struct vhost_dev *dev, unsigned long int
> > > > request,
> > > > + void *arg)
> > > > +{
> > > > +return vhost_vdpa_call_fd(vhost_vdpa_dev_fd(dev), request, arg);
> > > > +}
> > > > +
> > > > +static int vhost_vdpa_add_status_fd(int fd, uint8_t status)
> > > > {
> > > > uint8_t s;
> > > > int ret;
> > > >
> > > > -trace_vhost_vdpa_add_status(dev, status);
> > > > -ret = vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &s);
> > > > +ret = vhost_vdpa_call_fd(fd, VHOST_VDPA_GET_STATUS, &s);
> > > > if (ret < 0) {
> > > > return ret;
> > > > }
> > > >
> > > > s |= status;
> > > >
> > > > -ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s);
> > > > +ret = vhost_vdpa_call_fd(fd, VHOST_VDPA_SET_STATUS, &s);
> > > > if (ret < 0) {
> > > > return ret;
> > > > }
> > > >
> > > > -ret = vhost_vdpa_call(dev, VHOST_VDPA_GET_STATUS, &s);
> > > > +ret = vhost_vdpa_call_fd(fd, VHOST_VDPA_GET_STATUS, &s);
> > > > if (ret < 0) {
> > > > return ret;
> > > > }
> > > > @@ -378,6 +385,12 @@ static int vhost_vdpa_add_status(struct vhost_dev
> > > > *dev, uint8_t status)
> > > > return 0;
> > > > }
> > > >
> > > > +static int vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status)
> > > > +{
> > > > +trace_vhost_vdpa_add_status(dev, status);
> > > > +return vhost_vdpa_add_status_fd(vhost_vdpa_dev_fd(dev), status);
> > > > +}
> > > > +
> > > > int vhost_vdpa_get_iova_range(int fd, struct vhost_vdpa_iova_range
> > > > *iova_range)
> > > > {
> > > > int ret = ioctl(fd, VHOST_VDPA_GET_IOVA_RANGE, iova_range);
> > > > @@ -709,16 +722,20 @@ static int vhost_vdpa_get_device_id(struct
> > > > vhost_dev *dev,
> > > > return ret;
> > > > }
> > > >
> > > > +static int vhost_vdpa_reset_device_fd(int fd)
> > > > +{
> > > > +uint8_t status = 0;
> > > > +
> > > > +return vhost_vd
Re: [PATCH v3] hw/riscv: virt: Assume M-mode FW in pflash0 only when "-bios none"
Hi Andrea,
On Wed, May 24, 2023 at 03:50:34PM +, Andrea Bolognani wrote:
> On Tue, May 23, 2023 at 03:58:05PM +0530, Sunil V L wrote:
> > Currently, virt machine supports two pflash instances each with
> > 32MB size. However, the first pflash is always assumed to
> > contain M-mode firmware and reset vector is set to this if
> > enabled. Hence, for S-mode payloads like EDK2, only one pflash
> > instance is available for use. This means both code and NV variables
> > of EDK2 will need to use the same pflash.
> >
> > The OS distros keep the EDK2 FW code as readonly. When non-volatile
> > variables also need to share the same pflash, it is not possible
> > to keep it as readonly since variables need write access.
> >
> > To resolve this issue, the code and NV variables need to be separated.
> > But in that case we need an extra flash. Hence, modify the convention
> > such that pflash0 will contain the M-mode FW only when "-bios none"
> > option is used. Otherwise, pflash0 will contain the S-mode payload FW.
> > This enables both pflash instances available for EDK2 use.
> >
> > Example usage:
> > 1) pflash0 containing M-mode FW
> > qemu-system-riscv64 -bios none -pflash -machine virt
> > or
> > qemu-system-riscv64 -bios none \
> > -drive file=,if=pflash,format=raw,unit=0 -machine virt
> >
> > 2) pflash0 containing S-mode payload like EDK2
> > qemu-system-riscv64 -pflash -pflash -machine
> > virt
> > or
> > qemu-system-riscv64 -bios \
> > -pflash \
> > -pflash \
> > -machine virt
> > or
> > qemu-system-riscv64 -bios \
> > -drive file=,if=pflash,format=raw,unit=0,readonly=on \
> > -drive file=,if=pflash,format=raw,unit=1 \
> > -machine virt
>
> I wanted to test this, both directly with QEMU and through libvirt,
Thank you very much for helping with this!
> but I'm hitting two roadblocks.
>
> First off, the only RISC-V edk2 build readily accessible to me (from
> the edk2-riscv64 Fedora package) is configured to work off a R/W
> pflash1. You said that you have edk2 patches making R/O CODE pflash0
> and R/W VARS pflash1 ready. Any chance you could make either the
> build output, or the patches and some hints on how to build edk2
> after applying them, somewhere?
>
Please build EDK2 using the branch
https://github.com/vlsunil/edk2/tree/separate_code_vars.
The instructions to build is in
https://github.com/vlsunil/riscv-uefi-edk2-docs/wiki/RISC-V-Qemu-Virt-support#build-edk2
However, now it will create two images for code and vars.
> Going further and testing libvirt integration. After hacking around
> other issues, I finally stumbled upon this error:
>
> qemu-system-riscv64: Property 'virt-machine.pflash0' not found
>
> This is because a few years back libvirt has stopped using -drive
> if=pflash for configuring pflash devices, and these days it generates
> something along the lines of
>
> -blockdev
> '{"driver":"file","filename":"...","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
> -blockdev
> '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
> -machine virt,pflash0=libvirt-pflash0-format
>
> instead, which both aarch64/virt and x86_64/q35 machine types are
> perfectly happy with.
>
> I've tried to patch hw/riscv/virt.c to get the above working, and
> almost managed to succeed :) but eventually my unfamiliarity with the
> internals of QEMU caught up with me. Can you please look into making
> this work?
>
Thanks!. This needs some investigation. Let me look into supporting
this.
Thanks!
Sunil
[PATCH] meson.build: Fix glib -Wno-unused-function workaround
We want to only enable '-Wno-unused-function' if glib's version is
smaller than '2.57.2' and has a G_DEFINE_AUTOPTR_CLEANUP_FUNC()
implementation that doesn't take into account unused functions. But the
compilation test isn't working as intended as '-Wunused-function' isn't
enabled while running it.
Let's enable it.
Fixes: fc9a809e0d28 ("build: move glib detection and workarounds to meson")
Signed-off-by: Nicolas Saenz Julienne
---
Meson logs before:
Running compile:
Working directory: /local/home/nsaenz/c/qemu/build/meson-private/tmp5fgb3xnk
Command line: clang -m64 -mcx16 -I/usr/include/glib-2.0
-I/usr/lib64/glib-2.0/include
/local/home/nsaenz/c/qemu/build/meson-private/tmp5fgb3xnk/testfile.c -o
/local/home/nsaenz/c/qemu/build/meson-private/tmp5fgb3xnk/output.obj -c
-D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration
-Werror=unknown-warning-option -Werror=unused-command-line-argument
-Werror=ignored-optimization-argument -std=gnu11 -Werror
Code:
#include
typedef struct Foo {
int i;
} Foo;
static void foo_free(Foo *f)
{
g_free(f);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
int main(void) { return 0; }
Compiler stdout:
Compiler stderr:
Meson logs after:
Running compile:
Working directory: /local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u
Command line: clang -m64 -mcx16 -I/usr/include/glib-2.0
-I/usr/lib64/glib-2.0/include
/local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u/testfile.c -o
/local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u/output.obj -c
-D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration
-Werror=unknown-warning-option -Werror=unused-command-line-argument
-Werror=ignored-optimization-argument -std=gnu11 -Wunused-function -Werror
Code:
#include
typedef struct Foo {
int i;
} Foo;
static void foo_free(Foo *f)
{
g_free(f);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
int main(void) { return 0; }
Compiler stdout:
Compiler stderr:
/local/home/nsaenz/c/qemu/build/meson-private/tmp1fnp2s4u/testfile.c:10:3:
error: unused function 'glib_autoptr_cleanup_Foo' [-Werror,-Wunused-function]
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
^
/usr/include/glib-2.0/glib/gmacros.h:461:22: note: expanded from macro
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
static inline void _GLIB_AUTOPTR_FUNC_NAME(TypeName) (TypeName **_ptr) { if
(*_ptr) (func) (*_ptr); } \
^
/usr/include/glib-2.0/glib/gmacros.h:441:43: note: expanded from macro
'_GLIB_AUTOPTR_FUNC_NAME'
#define _GLIB_AUTOPTR_FUNC_NAME(TypeName) glib_autoptr_cleanup_##TypeName
^
:58:1: note: expanded from here
glib_autoptr_cleanup_Foo
^
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index ef181ff2df..71a4fcee52 100644
--- a/meson.build
+++ b/meson.build
@@ -780,7 +780,7 @@ if not cc.compiles('''
g_free(f);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
- int main(void) { return 0; }''', dependencies: glib_pc, args: ['-Werror'])
+ int main(void) { return 0; }''', dependencies: glib_pc, args:
['-Wunused-function', '-Werror'])
glib_cflags += cc.get_supported_arguments('-Wno-unused-function')
endif
glib = declare_dependency(dependencies: [glib_pc, gmodule],
--
2.39.2
Re: [PATCH 1/1] Hexagon (target/hexagon) Change Hexagon maintainer
Taylor Simpson writes: > Change Hexagon maintainer from Taylor Simpson to Brian Cain > Put Taylor's gmail address in .mailmap > > Signed-off-by: Taylor Simpson Thanks for all your efforts getting in Hexagon up-streamed and supporting it over the years. Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] checkpatch: Prefer DEFINE_TYPES() over type_init / type_register_static
On 5/24/23 07:54, Philippe Mathieu-Daudé wrote: When multiple QOM types are registered in the same file, it is clearer and simpler to use the the DEFINE_TYPES() macro. Add a rule to checkpatch.pl to suggest using DEFINE_TYPES() instead of type_init() / type_register_static(). Suggested-by: Daniel Henrique Barboza Signed-off-by: Philippe Mathieu-Daudé --- scripts/checkpatch.pl | 6 ++ 1 file changed, 6 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v12 02/15] accel: collecting TB execution count
On 5/24/23 06:35, Wu, Fei wrote:
On 5/23/2023 8:45 AM, Richard Henderson wrote:
On 5/18/23 06:57, Fei Wu wrote:
+void HELPER(inc_exec_freq)(void *ptr)
+{
+ TBStatistics *stats = (TBStatistics *) ptr;
+ tcg_debug_assert(stats);
+ ++stats->executions.normal;
+}
...
+static inline void gen_tb_exec_count(TranslationBlock *tb)
+{
+ if (tb_stats_enabled(tb, TB_EXEC_STATS)) {
+ TCGv_ptr ptr = tcg_temp_new_ptr();
+ tcg_gen_movi_ptr(ptr, (intptr_t)tb->tb_stats);
+ gen_helper_inc_exec_freq(ptr);
+ }
+}
This is 3 host instructions, easily expanded inline:
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -11,6 +11,7 @@
#include "qemu/error-report.h"
#include "tcg/tcg.h"
#include "tcg/tcg-op.h"
+#include "tcg/tcg-temp-internal.h"
#include "exec/exec-all.h"
#include "exec/gen-icount.h"
#include "exec/log.h"
@@ -18,6 +19,30 @@
#include "exec/plugin-gen.h"
#include "exec/replay-core.h"
+
+static void gen_tb_exec_count(TranslationBlock *tb)
+{
+ if (tb_stats_enabled(tb, TB_EXEC_STATS)) {
+ TCGv_ptr ptr = tcg_temp_ebb_new_ptr();
+
+ tcg_gen_movi_ptr(ptr, (intptr_t)&tb->tb_stats->executions.normal);
+ if (sizeof(tb->tb_stats->executions.normal) == 4) {
+ TCGv_i32 t = tcg_temp_ebb_new_i32();
+ tcg_gen_ld_i32(t, ptr, 0);
+ tcg_gen_addi_i32(t, t, 1);
+ tcg_gen_st_i32(t, ptr, 0);
+ tcg_temp_free_i32(t);
+ } else {
+ TCGv_i64 t = tcg_temp_ebb_new_i64();
+ tcg_gen_ld_i64(t, ptr, 0);
+ tcg_gen_addi_i64(t, t, 1);
+ tcg_gen_st_i64(t, ptr, 0);
+ tcg_temp_free_i64(t);
+ }
+ tcg_temp_free_ptr(ptr);
+ }
+}
+
bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
{
/* Suppress goto_tb if requested. */
I'm not expecially keen on embedding the TBStatistics pointer directly
like this; for most hosts we will have to put this constant into the
constant pool. Whereas the pointer already exists at tb->tb_stats, and
tb is at a constant displacement prior to the code, so we already have
mechanisms for generating pc-relative addresses.
However, that's premature optimization. Let's get it working first.
Richard, have you reviewed the whole series? I will integrate your
change to next version.
No, it's difficult to see what's going on.
In your next revision, please remove CONFIG_PROFILER entirely first, which was what I was
planning to do locally.
r~
Re: [PATCH] intel_iommu: Optimize out some unnecessary UNMAP calls
Hi, Zhenzhong,
On Tue, May 23, 2023 at 04:07:02PM +0800, Zhenzhong Duan wrote:
> Commit 63b88968f1 ("intel-iommu: rework the page walk logic") adds logic
> to record mapped IOVA ranges so we only need to send MAP or UNMAP when
> necessary. But there are still a few corner cases of unnecessary UNMAP.
>
> One is address space switch. During switching to iommu address space,
> all the original mappings have been dropped by VFIO memory listener,
> we don't need to unmap again in replay. The other is invalidation,
> we only need to unmap when there are recorded mapped IOVA ranges,
> presuming most of OSes allocating IOVA range continuously,
> ex. on x86, linux sets up mapping from 0x downwards.
>
> Signed-off-by: Zhenzhong Duan
> ---
> Tested on x86 with a net card passed or hotpluged to kvm guest,
> ping/ssh pass.
Since this is a performance related patch, do you have any number to show
the effect?
>
> hw/i386/intel_iommu.c | 31 ++-
> 1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 94d52f4205d2..6afd6428 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3743,6 +3743,7 @@ static void vtd_address_space_unmap(VTDAddressSpace
> *as, IOMMUNotifier *n)
> hwaddr start = n->start;
> hwaddr end = n->end;
> IntelIOMMUState *s = as->iommu_state;
> +IOMMUTLBEvent event;
> DMAMap map;
>
> /*
> @@ -3762,22 +3763,25 @@ static void vtd_address_space_unmap(VTDAddressSpace
> *as, IOMMUNotifier *n)
> assert(start <= end);
> size = remain = end - start + 1;
>
> +event.type = IOMMU_NOTIFIER_UNMAP;
> +event.entry.target_as = &address_space_memory;
> +event.entry.perm = IOMMU_NONE;
> +/* This field is meaningless for unmap */
> +event.entry.translated_addr = 0;
> +
> while (remain >= VTD_PAGE_SIZE) {
> -IOMMUTLBEvent event;
> uint64_t mask = dma_aligned_pow2_mask(start, end, s->aw_bits);
> uint64_t size = mask + 1;
>
> assert(size);
>
> -event.type = IOMMU_NOTIFIER_UNMAP;
> -event.entry.iova = start;
> -event.entry.addr_mask = mask;
> -event.entry.target_as = &address_space_memory;
> -event.entry.perm = IOMMU_NONE;
> -/* This field is meaningless for unmap */
> -event.entry.translated_addr = 0;
> -
> -memory_region_notify_iommu_one(n, &event);
> +map.iova = start;
> +map.size = size;
> +if (iova_tree_find(as->iova_tree, &map)) {
> +event.entry.iova = start;
> +event.entry.addr_mask = mask;
> +memory_region_notify_iommu_one(n, &event);
> +}
This one looks fine to me, but I'm not sure how much benefit we'll get here
either as this path should be rare afaiu.
>
> start += size;
> remain -= size;
> @@ -3826,13 +3830,6 @@ static void vtd_iommu_replay(IOMMUMemoryRegion
> *iommu_mr, IOMMUNotifier *n)
> uint8_t bus_n = pci_bus_num(vtd_as->bus);
> VTDContextEntry ce;
>
> -/*
> - * The replay can be triggered by either a invalidation or a newly
> - * created entry. No matter what, we release existing mappings
> - * (it means flushing caches for UNMAP-only registers).
> - */
> -vtd_address_space_unmap(vtd_as, n);
IIUC this is needed to satisfy current replay() semantics:
/**
* @replay:
*
* Called to handle memory_region_iommu_replay().
*
* The default implementation of memory_region_iommu_replay() is to
* call the IOMMU translate method for every page in the address space
* with flag == IOMMU_NONE and then call the notifier if translate
* returns a valid mapping. If this method is implemented then it
* overrides the default behaviour, and must provide the full semantics
* of memory_region_iommu_replay(), by calling @notifier for every
* translation present in the IOMMU.
The problem is vtd_page_walk() currently by default only notifies on page
changes, so we'll notify all MAP only if we unmap all of them first.
I assumed it was not a major issue with/without it before because
previously AFAIU the major path to trigger this is when someone hot plug a
vfio-pci into an existing guest IOMMU domain, so the unmap_all() is indeed
no-op. However from semantics level it seems unmap_all() is still needed.
The other thing is when I am looking at the new code I found that we
actually extended the replay() to be used also in dirty tracking of vfio,
in vfio_sync_dirty_bitmap(). For that maybe it's already broken if
unmap_all() because afaiu log_sync() can be called in migration thread
anytime during DMA so I think it means the device is prone to DMA with the
IOMMU pgtable quickly erased and rebuilt here, which means the DMA could
fail unexpectedly. Copy Alex, Kirti and Neo.
Perhaps to fix it we'll need to teach the vtd pgtable walker to notify all
existin
[PATCH 1/1] Hexagon (target/hexagon) Change Hexagon maintainer
Change Hexagon maintainer from Taylor Simpson to Brian Cain Put Taylor's gmail address in .mailmap Signed-off-by: Taylor Simpson --- MAINTAINERS | 2 +- .mailmap| 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 1b6466496d..426d33f4cb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -215,7 +215,7 @@ F: tests/tcg/cris/ F: disas/cris.c Hexagon TCG CPUs -M: Taylor Simpson +M: Brian Cain S: Supported F: target/hexagon/ X: target/hexagon/idef-parser/ diff --git a/.mailmap b/.mailmap index bbe6d3fd69..b57da4827e 100644 --- a/.mailmap +++ b/.mailmap @@ -78,6 +78,7 @@ Philippe Mathieu-Daudé Philippe Mathieu-Daudé Stefan Brankovic Yongbok Kim +Taylor Simpson # Also list preferred name forms where people have changed their # git author config, or had utf8/latin1 encoding issues. -- 2.25.1
[PATCH 0/1] Change Hexagon maintainer
I will be retiring from Qualcomm at the end of May, so I am placing the maintainership in the very capable hands of Brian Cain. Taylor Simpson (1): Hexagon (target/hexagon) Change Hexagon maintainer MAINTAINERS | 2 +- .mailmap| 1 + 2 files changed, 2 insertions(+), 1 deletion(-) -- 2.25.1
RE: [PATCH v2 2/2] Hexagon: fix outdated `hex_new_*` comments
> -Original Message- > From: Matheus Tavares Bernardino > Sent: Wednesday, May 24, 2023 9:42 AM > To: qemu-devel@nongnu.org > Cc: Taylor Simpson ; Brian Cain > ; Marco Liebel (QUIC) > Subject: [PATCH v2 2/2] Hexagon: fix outdated `hex_new_*` comments > > Some code comments refer to hex_new_value and hex_new_pred_value, > which have been transferred to DisasContext and, in the case of > hex_new_value, should now be accessed through get_result_gpr(). > > In order to fix this outdated comments and also avoid having to tweak them > whenever we make a variable name change in the future, let's replace them > with pseudocode. > > Suggested-by: Taylor Simpson > Signed-off-by: Matheus Tavares Bernardino > --- > target/hexagon/genptr.c| 26 -- > target/hexagon/translate.c | 2 +- > 2 files changed, 13 insertions(+), 15 deletions(-) Reviewed-by: Taylor Simpson Queued to hex.next
RE: [PATCH v2 1/2] target/hexagon/*.py: clean up used 'toss' and 'numregs' vars
> -Original Message- > From: Matheus Tavares Bernardino > Sent: Wednesday, May 24, 2023 9:42 AM > To: qemu-devel@nongnu.org > Cc: Taylor Simpson ; Brian Cain > ; Marco Liebel (QUIC) ; > Alessandro Di Federico ; Anton Johansson > Subject: [PATCH v2 1/2] target/hexagon/*.py: clean up used 'toss' and > 'numregs' vars > > Many Hexagon python scripts call hex_common.get_tagregs(), but only one > call site use the full reg structure given by this function. To make the code > cleaner, let's make get_tagregs() filter out the unused fields (i.e. 'toss' > and > 'numregs'), properly removed the unused variables at the call sites. The > hex_common.bad_register() function is also adjusted to work exclusively with > 'regtype' and 'regid' args. For the single call site that does use > toss/numregs, > we provide an optional parameter to > get_tagregs() which will restore the old full behavior. > > Suggested-by: Taylor Simpson > Signed-off-by: Matheus Tavares Bernardino > --- > target/hexagon/gen_analyze_funcs.py | 10 +++--- > target/hexagon/gen_helper_funcs.py | 30 > target/hexagon/gen_helper_protos.py | 22 ++-- > target/hexagon/gen_idef_parser_funcs.py | 4 +-- > target/hexagon/gen_op_regs.py | 4 +-- > target/hexagon/gen_tcg_funcs.py | 46 - > target/hexagon/hex_common.py| 24 ++--- > 7 files changed, 70 insertions(+), 70 deletions(-) Reviewed-by: Taylor Simpson Tested-by: Taylor Simpson Queued to hex.next
Re: [RFC PATCH 4/6] Convert query-block/info_block to coroutine
On 5/24/23 11:24, Lin Ma wrote: > The query-named-block-nodes is only availabe for qmp, not support hmp yet. > > Lin Ok, makes sense.
Re: [RFC PATCH 6/6] block: Add a thread-pool version of fstat
On 5/23/23 23:39, Fabiano Rosas wrote:
> From: João Silva
>
> The fstat call can take a long time to finish when running over
> NFS. Add a version of it that runs in the thread pool.
>
> Adapt one of its users, raw_co_get_allocated_file size to use the new
> version. That function is called via QMP under the qemu_global_mutex
> so it has a large chance of blocking VCPU threads in case it takes too
> long to finish.
>
> Signed-off-by: João Silva
> Signed-off-by: Fabiano Rosas
> ---
> block/file-posix.c | 40 +---
> include/block/raw-aio.h | 4 +++-
> 2 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 0ab158efba..0a29a6cc43 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -227,6 +227,9 @@ typedef struct RawPosixAIOData {
> struct {
> unsigned long op;
> } zone_mgmt;
> +struct {
> +struct stat *st;
> +} fstat;
> };
> } RawPosixAIOData;
>
> @@ -2644,6 +2647,34 @@ static void raw_close(BlockDriverState *bs)
> }
> }
>
> +static int handle_aiocb_fstat(void *opaque)
> +{
> +RawPosixAIOData *aiocb = opaque;
> +
> +if (fstat(aiocb->aio_fildes, aiocb->fstat.st) < 0) {
> +return -errno;
> +}
> +
> +return 0;
> +}
> +
> +static int coroutine_fn raw_co_fstat(BlockDriverState *bs, struct stat *st)
> +{
> +BDRVRawState *s = bs->opaque;
> +RawPosixAIOData acb;
> +
> +acb = (RawPosixAIOData) {
> +.bs = bs,
> +.aio_fildes = s->fd,
> +.aio_type = QEMU_AIO_FSTAT,
> +.fstat = {
> +.st = st,
> +},
> +};
> +
> +return raw_thread_pool_submit(handle_aiocb_fstat, &acb);
> +}
> +
> /**
> * Truncates the given regular file @fd to @offset and, when growing, fills
> the
> * new space according to @prealloc.
> @@ -2883,11 +2914,14 @@ static int64_t coroutine_fn
> raw_co_getlength(BlockDriverState *bs)
> static int64_t coroutine_fn raw_co_get_allocated_file_size(BlockDriverState
> *bs)
> {
> struct stat st;
> -BDRVRawState *s = bs->opaque;
> +int ret;
>
> -if (fstat(s->fd, &st) < 0) {
> -return -errno;
> +ret = raw_co_fstat(bs, &st);
> +
> +if (ret) {
> +return ret;
> }
> +
> return (int64_t)st.st_blocks * 512;
> }
>
> diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
> index 0fe85ade77..7dc6d24e21 100644
> --- a/include/block/raw-aio.h
> +++ b/include/block/raw-aio.h
> @@ -31,6 +31,7 @@
> #define QEMU_AIO_ZONE_REPORT 0x0100
> #define QEMU_AIO_ZONE_MGMT0x0200
> #define QEMU_AIO_ZONE_APPEND 0x0400
> +#define QEMU_AIO_FSTAT0x0800
> #define QEMU_AIO_TYPE_MASK \
> (QEMU_AIO_READ | \
> QEMU_AIO_WRITE | \
> @@ -42,7 +43,8 @@
> QEMU_AIO_TRUNCATE | \
> QEMU_AIO_ZONE_REPORT | \
> QEMU_AIO_ZONE_MGMT | \
> - QEMU_AIO_ZONE_APPEND)
> + QEMU_AIO_ZONE_APPEND | \
> + QEMU_AIO_FSTAT)
>
> /* AIO flags */
> #define QEMU_AIO_MISALIGNED 0x1000
Reviewed-by: Claudio Fontana
