Re: [PATCH 0/4] Fix some style problems in qobject
Patchew URL:
https://patchew.org/QEMU/20201228071129.24563-1-zhangha...@huawei.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201228071129.24563-1-zhangha...@huawei.com
Subject: [PATCH 0/4] Fix some style problems in qobject
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update]
patchew/20201219104229.1964-1-mark.cave-ayl...@ilande.co.uk ->
patchew/20201219104229.1964-1-mark.cave-ayl...@ilande.co.uk
- [tag update] patchew/20201226103347.868-1-gaojin...@huawei.com ->
patchew/20201226103347.868-1-gaojin...@huawei.com
* [new tag] patchew/20201228071129.24563-1-zhangha...@huawei.com ->
patchew/20201228071129.24563-1-zhangha...@huawei.com
Switched to a new branch 'test'
1caa0fd qobject: braces {} are necessary for all arms of this statement
b0ca07e qobject: spaces required around that operators
0adae60 qobject: code indent should never use tabs
4eea991 qobject: open brace '{' following struct go on the same line
=== OUTPUT BEGIN ===
1/4 Checking commit 4eea9919361d (qobject: open brace '{' following struct go
on the same line)
2/4 Checking commit 0adae6079b21 (qobject: code indent should never use tabs)
3/4 Checking commit b0ca07e6cac6 (qobject: spaces required around that
operators)
ERROR: braces {} are necessary for all arms of this statement
#22: FILE: qobject/qdict.c:45:
+for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++)
[...]
total: 1 errors, 0 warnings, 10 lines checked
Patch 3/4 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/4 Checking commit 1caa0fdbcda9 (qobject: braces {} are necessary for all arms
of this statement)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20201228071129.24563-1-zhangha...@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 5/6] spapr: Add drc_ prefix to the DRC realize and unrealize functions
On Fri, Dec 18, 2020 at 11:33:59AM +0100, Greg Kurz wrote:
> Use a less generic name for an easier experience with tools such as
> cscope or grep.
>
> Signed-off-by: Greg Kurz
Applied to ppc-for-6.0, thanks.
> ---
> hw/ppc/spapr_drc.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index a4d2608017c5..8571d5bafe4e 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -503,7 +503,7 @@ static const VMStateDescription vmstate_spapr_drc = {
> }
> };
>
> -static void realize(DeviceState *d, Error **errp)
> +static void drc_realize(DeviceState *d, Error **errp)
> {
> SpaprDrc *drc = SPAPR_DR_CONNECTOR(d);
> Object *root_container;
> @@ -530,7 +530,7 @@ static void realize(DeviceState *d, Error **errp)
> trace_spapr_drc_realize_complete(spapr_drc_index(drc));
> }
>
> -static void unrealize(DeviceState *d)
> +static void drc_unrealize(DeviceState *d)
> {
> SpaprDrc *drc = SPAPR_DR_CONNECTOR(d);
> Object *root_container;
> @@ -579,8 +579,8 @@ static void spapr_dr_connector_class_init(ObjectClass *k,
> void *data)
> {
> DeviceClass *dk = DEVICE_CLASS(k);
>
> -dk->realize = realize;
> -dk->unrealize = unrealize;
> +dk->realize = drc_realize;
> +dk->unrealize = drc_unrealize;
> /*
> * Reason: DR connector needs to be wired to either the machine or to a
> * PHB in spapr_dr_connector_new().
> @@ -628,7 +628,7 @@ static void realize_physical(DeviceState *d, Error **errp)
> SpaprDrcPhysical *drcp = SPAPR_DRC_PHYSICAL(d);
> Error *local_err = NULL;
>
> -realize(d, _err);
> +drc_realize(d, _err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> @@ -644,7 +644,7 @@ static void unrealize_physical(DeviceState *d)
> {
> SpaprDrcPhysical *drcp = SPAPR_DRC_PHYSICAL(d);
>
> -unrealize(d);
> +drc_unrealize(d);
> vmstate_unregister(VMSTATE_IF(drcp), _spapr_drc_physical, drcp);
> qemu_unregister_reset(drc_physical_reset, drcp);
> }
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 3/6] spapr: Introduce spapr_drc_reset_all()
On Fri, Dec 18, 2020 at 11:33:57AM +0100, Greg Kurz wrote:
> No need to expose the way DRCs are traversed outside of spapr_drc.c.
>
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> include/hw/ppc/spapr_drc.h | 6 ++
> hw/ppc/spapr_drc.c | 31 +
> hw/ppc/spapr_hcall.c | 40 ++
> 3 files changed, 43 insertions(+), 34 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 5d80019f82e2..8982927d5c24 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -245,6 +245,12 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner,
> uint32_t drc_type_mask);
> void spapr_drc_attach(SpaprDrc *drc, DeviceState *d);
> void spapr_drc_detach(SpaprDrc *drc);
>
> +/*
> + * Reset all DRCs, causing pending hot-plug/unplug requests to complete.
> + * Safely handles potential DRC removal (eg. PHBs or PCI bridges).
> + */
> +void spapr_drc_reset_all(struct SpaprMachineState *spapr);
> +
> static inline bool spapr_drc_unplug_requested(SpaprDrc *drc)
> {
> return drc->unplug_requested;
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 5b5e2ac58a7e..a4d2608017c5 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -949,6 +949,37 @@ out:
> return ret;
> }
>
> +void spapr_drc_reset_all(SpaprMachineState *spapr)
> +{
> +Object *drc_container;
> +ObjectProperty *prop;
> +ObjectPropertyIterator iter;
> +
> +drc_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
> +restart:
> +object_property_iter_init(, drc_container);
> +while ((prop = object_property_iter_next())) {
> +SpaprDrc *drc;
> +
> +if (!strstart(prop->type, "link<", NULL)) {
> +continue;
> +}
> +drc = SPAPR_DR_CONNECTOR(object_property_get_link(drc_container,
> + prop->name,
> + _abort));
> +
> +/*
> + * This will complete any pending plug/unplug requests.
> + * In case of a unplugged PHB or PCI bridge, this will
> + * cause some DRCs to be destroyed and thus potentially
> + * invalidate the iterator.
> + */
> +if (spapr_drc_reset(drc)) {
> +goto restart;
> +}
> +}
> +}
> +
> /*
> * RTAS calls
> */
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index aa22830ac4bd..e5dfc1ba7acc 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1632,39 +1632,6 @@ static uint32_t cas_check_pvr(PowerPCCPU *cpu,
> uint32_t max_compat,
> return best_compat;
> }
>
> -static void spapr_handle_transient_dev_before_cas(SpaprMachineState *spapr)
> -{
> -Object *drc_container;
> -ObjectProperty *prop;
> -ObjectPropertyIterator iter;
> -
> -drc_container = container_get(object_get_root(), "/dr-connector");
> -restart:
> -object_property_iter_init(, drc_container);
> -while ((prop = object_property_iter_next())) {
> -SpaprDrc *drc;
> -
> -if (!strstart(prop->type, "link<", NULL)) {
> -continue;
> -}
> -drc = SPAPR_DR_CONNECTOR(object_property_get_link(drc_container,
> - prop->name,
> - _abort));
> -
> -/*
> - * This will complete any pending plug/unplug requests.
> - * In case of a unplugged PHB or PCI bridge, this will
> - * cause some DRCs to be destroyed and thus potentially
> - * invalidate the iterator.
> - */
> -if (spapr_drc_reset(drc)) {
> -goto restart;
> -}
> -}
> -
> -spapr_clear_pending_hotplug_events(spapr);
> -}
> -
> target_ulong do_client_architecture_support(PowerPCCPU *cpu,
> SpaprMachineState *spapr,
> target_ulong vec,
> @@ -1822,7 +1789,12 @@ target_ulong do_client_architecture_support(PowerPCCPU
> *cpu,
>
> spapr_irq_update_active_intc(spapr);
>
> -spapr_handle_transient_dev_before_cas(spapr);
> +/*
> + * Process all pending hot-plug/unplug requests now. An updated full
> + * rendered FDT will be returned to the guest.
> + */
> +spapr_drc_reset_all(spapr);
> +spapr_clear_pending_hotplug_events(spapr);
>
> /*
> * If spapr_machine_reset() did not set up a HPT but one is necessary
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH] spapr: Fix buffer overflow in spapr_numa_associativity_init()
On Fri, Dec 18, 2020 at 02:53:24PM +0100, Greg Kurz wrote:
> Running a guest with 128 NUMA nodes crashes QEMU:
>
> ../../util/error.c:59: error_setv: Assertion `*errp == NULL' failed.
>
> The crash happens when setting the FWNMI migration blocker:
>
> 2861 if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI) == SPAPR_CAP_ON) {
> 2862 /* Create the error string for live migration blocker */
> 2863 error_setg(>fwnmi_migration_blocker,
> 2864 "A machine check is being handled during migration. The
> handler"
> 2865 "may run and log hardware error on the destination");
> 2866 }
>
> Inspection reveals that papr->fwnmi_migration_blocker isn't NULL:
>
> (gdb) p spapr->fwnmi_migration_blocker
> $1 = (Error *) 0x8400
>
> Since this is the only place where papr->fwnmi_migration_blocker is
> set, this means someone wrote there in our back. Further analysis
> points to spapr_numa_associativity_init(), especially the part
> that initializes the associative arrays for NVLink GPUs:
>
> max_nodes_with_gpus = nb_numa_nodes + NVGPU_MAX_NUM;
>
> ie. max_nodes_with_gpus = 128 + 6, but the array isn't sized to
> accommodate the 6 extra nodes:
>
> #define MAX_NODES 128
>
> struct SpaprMachineState {
> .
> .
> .
> uint32_t numa_assoc_array[MAX_NODES][NUMA_ASSOC_SIZE];
>
> Error *fwnmi_migration_blocker;
> };
>
> and the following loops happily overwrite spapr->fwnmi_migration_blocker,
> and probably more:
>
> for (i = nb_numa_nodes; i < max_nodes_with_gpus; i++) {
> spapr->numa_assoc_array[i][0] = cpu_to_be32(MAX_DISTANCE_REF_POINTS);
>
> for (j = 1; j < MAX_DISTANCE_REF_POINTS; j++) {
> uint32_t gpu_assoc = smc->pre_5_1_assoc_refpoints ?
> SPAPR_GPU_NUMA_ID : cpu_to_be32(i);
> spapr->numa_assoc_array[i][j] = gpu_assoc;
> }
>
> spapr->numa_assoc_array[i][MAX_DISTANCE_REF_POINTS] = cpu_to_be32(i);
> }
>
> Fix the size of the array. This requires "hw/ppc/spapr.h" to see
> NVGPU_MAX_NUM. Including "hw/pci-host/spapr.h" introduces a
> circular dependency that breaks the build, so this moves the
> definition of NVGPU_MAX_NUM to "hw/ppc/spapr.h" instead.
>
> Reported-by: Min Deng
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1908693
> Fixes: dd7e1d7ae431 ("spapr_numa: move NVLink2 associativity handling to
> spapr_numa.c")
> Cc: danielhb...@gmail.com
> Signed-off-by: Greg Kurz
Oof. Applied.
> ---
> include/hw/pci-host/spapr.h |2 --
> include/hw/ppc/spapr.h |5 -
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 4f58f0223b56..bd014823a933 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -115,8 +115,6 @@ struct SpaprPhbState {
> #define SPAPR_PCI_NV2RAM64_WIN_BASE SPAPR_PCI_LIMIT
> #define SPAPR_PCI_NV2RAM64_WIN_SIZE (2 * TiB) /* For up to 6 GPUs 256GB
> each */
>
> -/* Max number of these GPUsper a physical box */
> -#define NVGPU_MAX_NUM6
> /* Max number of NVLinks per GPU in any physical box */
> #define NVGPU_MAX_LINKS 3
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 06a5b4259f20..1cc19575f548 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -112,6 +112,9 @@ typedef enum {
> #define NUMA_ASSOC_SIZE(MAX_DISTANCE_REF_POINTS + 1)
> #define VCPU_ASSOC_SIZE(NUMA_ASSOC_SIZE + 1)
>
> +/* Max number of these GPUsper a physical box */
> +#define NVGPU_MAX_NUM6
> +
> typedef struct SpaprCapabilities SpaprCapabilities;
> struct SpaprCapabilities {
> uint8_t caps[SPAPR_CAP_NUM];
> @@ -240,7 +243,7 @@ struct SpaprMachineState {
> unsigned gpu_numa_id;
> SpaprTpmProxy *tpm_proxy;
>
> -uint32_t numa_assoc_array[MAX_NODES][NUMA_ASSOC_SIZE];
> +uint32_t numa_assoc_array[MAX_NODES + NVGPU_MAX_NUM][NUMA_ASSOC_SIZE];
>
> Error *fwnmi_migration_blocker;
> };
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 4/6] spapr: Use spapr_drc_reset_all() at machine reset
On Fri, Dec 18, 2020 at 11:33:58AM +0100, Greg Kurz wrote:
> Documentation of object_child_foreach_recursive() clearly stipulates
> that "it is forbidden to add or remove children from @obj from the @fn
> callback". But this is exactly what we do during machine reset. The call
> to spapr_drc_reset() can finalize the hot-unplug sequence of a PHB or a
> PCI bridge, both of which will then in turn destroy their PCI DRCs. This
> could potentially invalidate the iterator used by do_object_child_foreach().
> It is pure luck that this haven't caused any issues so far.
>
> Use spapr_drc_reset_all() since it can cope with DRC removal.
>
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> hw/ppc/spapr.c | 15 +--
> 1 file changed, 1 insertion(+), 14 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 43dded87f498..8528bc90fec4 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1566,19 +1566,6 @@ void spapr_setup_hpt(SpaprMachineState *spapr)
> }
> }
>
> -static int spapr_reset_drcs(Object *child, void *opaque)
> -{
> -SpaprDrc *drc =
> -(SpaprDrc *) object_dynamic_cast(child,
> - TYPE_SPAPR_DR_CONNECTOR);
> -
> -if (drc) {
> -spapr_drc_reset(drc);
> -}
> -
> -return 0;
> -}
> -
> static void spapr_machine_reset(MachineState *machine)
> {
> SpaprMachineState *spapr = SPAPR_MACHINE(machine);
> @@ -1633,7 +1620,7 @@ static void spapr_machine_reset(MachineState *machine)
> * will crash QEMU if the DIMM holding the vring goes away). To avoid
> such
> * situations, we reset DRCs after all devices have been reset.
> */
> -object_child_foreach_recursive(object_get_root(), spapr_reset_drcs,
> NULL);
> +spapr_drc_reset_all(spapr);
>
> spapr_clear_pending_events(spapr);
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 1/6] spapr: Call spapr_drc_reset() for all DRCs at CAS
On Fri, Dec 18, 2020 at 11:33:55AM +0100, Greg Kurz wrote:
> Non-transient DRCs are either in the empty or the ready state,
> which means spapr_drc_reset() doesn't change their state. It
> is thus not needed to do any checking. Call spapr_drc_reset()
> unconditionally and squash spapr_drc_transient() into its
> only user, spapr_drc_needed().
>
> Signed-off-by: Greg Kurz
Applied to ppc-fof-6.0, thanks.
> ---
> include/hw/ppc/spapr_drc.h | 3 ---
> hw/ppc/spapr_drc.c | 8 ++--
> hw/ppc/spapr_hcall.c | 7 ---
> 3 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index def3593adc8b..cff5e707d0d9 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -244,9 +244,6 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner,
> uint32_t drc_type_mask);
> void spapr_drc_attach(SpaprDrc *drc, DeviceState *d);
> void spapr_drc_detach(SpaprDrc *drc);
>
> -/* Returns true if a hot plug/unplug request is pending */
> -bool spapr_drc_transient(SpaprDrc *drc);
> -
> static inline bool spapr_drc_unplug_requested(SpaprDrc *drc)
> {
> return drc->unplug_requested;
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index fc7e321fcdf6..8d62f55066b6 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -462,8 +462,9 @@ static const VMStateDescription
> vmstate_spapr_drc_unplug_requested = {
> }
> };
>
> -bool spapr_drc_transient(SpaprDrc *drc)
> +static bool spapr_drc_needed(void *opaque)
> {
> +SpaprDrc *drc = opaque;
> SpaprDrcClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>
> /*
> @@ -483,11 +484,6 @@ bool spapr_drc_transient(SpaprDrc *drc)
> spapr_drc_unplug_requested(drc);
> }
>
> -static bool spapr_drc_needed(void *opaque)
> -{
> -return spapr_drc_transient(opaque);
> -}
> -
> static const VMStateDescription vmstate_spapr_drc = {
> .name = "spapr_drc",
> .version_id = 1,
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index c0ea0bd5794b..4e9d50c254f0 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1650,9 +1650,10 @@ static void
> spapr_handle_transient_dev_before_cas(SpaprMachineState *spapr)
>prop->name,
>_abort));
>
> -if (spapr_drc_transient(drc)) {
> -spapr_drc_reset(drc);
> -}
> +/*
> + * This will complete any pending plug/unplug requests.
> + */
> +spapr_drc_reset(drc);
> }
>
> spapr_clear_pending_hotplug_events(spapr);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 2/6] spapr: Fix reset of transient DR connectors
On Fri, Dec 18, 2020 at 11:33:56AM +0100, Greg Kurz wrote:
> Documentation of object_property_iter_init() clearly stipulates that
> "it is forbidden to modify the property list while iterating". But this
> is exactly what we do when resetting transient DR connectors during CAS.
> The call to spapr_drc_reset() can finalize the hot-unplug sequence of a
> PHB or a PCI bridge, both of which will then in turn destroy their PCI
> DRCs. This could potentially invalidate the iterator. It is pure luck
> that this haven't caused any issues so far.
>
> Change spapr_drc_reset() to return true if it caused a device to be
> removed. Restart from scratch in this case. This can potentially
> increase the overall DRC reset time, especially with a high maxmem
> which generates a lot of LMB DRCs. But this kind of setup is rare,
> and so is the use case of rebooting a guest while doing hot-unplug.
>
> Signed-off-by: Greg Kurz
Applied, thanks.
> ---
> include/hw/ppc/spapr_drc.h | 3 ++-
> hw/ppc/spapr_drc.c | 6 +-
> hw/ppc/spapr_hcall.c | 8 +++-
> 3 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index cff5e707d0d9..5d80019f82e2 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -224,7 +224,8 @@ static inline bool spapr_drc_hotplugged(DeviceState *dev)
> return dev->hotplugged && !runstate_check(RUN_STATE_INMIGRATE);
> }
>
> -void spapr_drc_reset(SpaprDrc *drc);
> +/* Returns true if an unplug request completed */
> +bool spapr_drc_reset(SpaprDrc *drc);
>
> uint32_t spapr_drc_index(SpaprDrc *drc);
> SpaprDrcType spapr_drc_type(SpaprDrc *drc);
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 8d62f55066b6..5b5e2ac58a7e 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -417,9 +417,10 @@ void spapr_drc_detach(SpaprDrc *drc)
> spapr_drc_release(drc);
> }
>
> -void spapr_drc_reset(SpaprDrc *drc)
> +bool spapr_drc_reset(SpaprDrc *drc)
> {
> SpaprDrcClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +bool unplug_completed = false;
>
> trace_spapr_drc_reset(spapr_drc_index(drc));
>
> @@ -428,6 +429,7 @@ void spapr_drc_reset(SpaprDrc *drc)
> */
> if (drc->unplug_requested) {
> spapr_drc_release(drc);
> +unplug_completed = true;
> }
>
> if (drc->dev) {
> @@ -444,6 +446,8 @@ void spapr_drc_reset(SpaprDrc *drc)
> drc->ccs_offset = -1;
> drc->ccs_depth = -1;
> }
> +
> +return unplug_completed;
> }
>
> static bool spapr_drc_unplug_requested_needed(void *opaque)
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 4e9d50c254f0..aa22830ac4bd 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1639,6 +1639,7 @@ static void
> spapr_handle_transient_dev_before_cas(SpaprMachineState *spapr)
> ObjectPropertyIterator iter;
>
> drc_container = container_get(object_get_root(), "/dr-connector");
> +restart:
> object_property_iter_init(, drc_container);
> while ((prop = object_property_iter_next())) {
> SpaprDrc *drc;
> @@ -1652,8 +1653,13 @@ static void
> spapr_handle_transient_dev_before_cas(SpaprMachineState *spapr)
>
> /*
> * This will complete any pending plug/unplug requests.
> + * In case of a unplugged PHB or PCI bridge, this will
> + * cause some DRCs to be destroyed and thus potentially
> + * invalidate the iterator.
> */
> -spapr_drc_reset(drc);
> +if (spapr_drc_reset(drc)) {
> +goto restart;
> +}
> }
>
> spapr_clear_pending_hotplug_events(spapr);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[PATCH 4/4] qobject: braces {} are necessary for all arms of this statement
Add braces {} for arms of if/for statement
Signed-off-by: Zhang Han
---
qobject/qdict.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 05ec950e05..0a49b787ab 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -42,8 +42,9 @@ static unsigned int tdb_hash(const char *name)
unsigned i; /* Used to cycle through random values. */
/* Set the initial value from the key size. */
-for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++)
+for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++) {
value = (value + (((const unsigned char *)name)[i] << (i * 5 % 24)));
+}
return (1103515243 * value + 12345);
}
@@ -92,8 +93,9 @@ static QDictEntry *qdict_find(const QDict *qdict,
QDictEntry *entry;
QLIST_FOREACH(entry, >table[bucket], next)
-if (!strcmp(entry->key, key))
+if (!strcmp(entry->key, key)) {
return entry;
+}
return NULL;
}
--
2.29.1.59.gf9b6481aed
Re: [PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device
On Sat, Dec 19, 2020 at 10:42:25AM +, Mark Cave-Ayland wrote:
Really needs a commit message.
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/misc/macio/macio.c | 20 +--
> hw/ppc/mac_oldworld.c | 66 +--
> include/hw/misc/macio/macio.h | 2 +-
> 3 files changed, 43 insertions(+), 45 deletions(-)
>
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index bb601f782c..cfb87da6c9 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -140,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error
> **errp)
> {
> MacIOState *s = MACIO(d);
> OldWorldMacIOState *os = OLDWORLD_MACIO(d);
> -DeviceState *pic_dev = DEVICE(os->pic);
> +DeviceState *pic_dev = DEVICE(>pic);
> Error *err = NULL;
> SysBusDevice *sysbus_dev;
>
> @@ -150,6 +150,14 @@ static void macio_oldworld_realize(PCIDevice *d, Error
> **errp)
> return;
> }
>
> +/* Heathrow PIC */
> +if (!qdev_realize(DEVICE(>pic), BUS(>macio_bus), errp)) {
> +return;
> +}
> +sysbus_dev = SYS_BUS_DEVICE(>pic);
> +memory_region_add_subregion(>bar, 0x0,
> +sysbus_mmio_get_region(sysbus_dev, 0));
> +
> qdev_prop_set_uint64(DEVICE(>cuda), "timebase-frequency",
> s->frequency);
> if (!qdev_realize(DEVICE(>cuda), BUS(>macio_bus), errp)) {
> @@ -175,11 +183,6 @@ static void macio_oldworld_realize(PCIDevice *d, Error
> **errp)
> sysbus_mmio_get_region(sysbus_dev, 0));
> pmac_format_nvram_partition(>nvram, os->nvram.size);
>
> -/* Heathrow PIC */
> -sysbus_dev = SYS_BUS_DEVICE(os->pic);
> -memory_region_add_subregion(>bar, 0x0,
> -sysbus_mmio_get_region(sysbus_dev, 0));
> -
> /* IDE buses */
> macio_realize_ide(s, >ide[0],
>qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
> @@ -218,10 +221,7 @@ static void macio_oldworld_init(Object *obj)
> DeviceState *dev;
> int i;
>
> -object_property_add_link(obj, "pic", TYPE_HEATHROW,
> - (Object **) >pic,
> - qdev_prop_allow_set_link_before_realize,
> - 0);
> +object_initialize_child(OBJECT(s), "pic", >pic, TYPE_HEATHROW);
>
> object_initialize_child(OBJECT(s), "cuda", >cuda, TYPE_CUDA);
>
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index e58e0525fe..44ee99be88 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
> MACIOIDEState *macio_ide;
> ESCCState *escc;
> SysBusDevice *s;
> -DeviceState *dev, *pic_dev;
> +DeviceState *dev, *pic_dev, *grackle_dev;
> BusState *adb_bus;
> uint64_t bios_addr;
> int bios_size;
> @@ -227,10 +227,17 @@ static void ppc_heathrow_init(MachineState *machine)
> }
> }
>
> +/* Timebase Frequency */
> +if (kvm_enabled()) {
> +tbfreq = kvmppc_get_tbfreq();
> +} else {
> +tbfreq = TBFREQ;
> +}
> +
> /* Grackle PCI host bridge */
> -dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> -qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
> -s = SYS_BUS_DEVICE(dev);
> +grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> +qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x8000);
> +s = SYS_BUS_DEVICE(grackle_dev);
> sysbus_realize_and_unref(s, _fatal);
>
> sysbus_mmio_map(s, 0, GRACKLE_BASE);
> @@ -242,14 +249,30 @@ static void ppc_heathrow_init(MachineState *machine)
> memory_region_add_subregion(get_system_memory(), 0xfe00,
> sysbus_mmio_get_region(s, 3));
>
> -/* XXX: we register only 1 output pin for heathrow PIC */
> -pic_dev = qdev_new(TYPE_HEATHROW);
> -sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), _fatal);
> +pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
> +
> +/* MacIO */
> +macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
> +dev = DEVICE(macio);
> +qdev_prop_set_uint64(dev, "frequency", tbfreq);
> +
> +escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
> +qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
> +qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
> +
> +pci_realize_and_unref(macio, pci_bus, _fatal);
> +
> +pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
> +for (i = 0; i < 4; i++) {
> +qdev_connect_gpio_out(grackle_dev, i,
> + qdev_get_gpio_in(pic_dev, 0x15 + i));
> +}
>
> /* Connect the heathrow PIC outputs to the 6xx bus */
> for (i = 0; i < smp_cpus; i++) {
> switch (PPC_INPUT(env)) {
> case PPC_FLAGS_INPUT_6xx:
> +/* XXX: we register only 1 output pin for heathrow PIC
[PATCH 3/4] qobject: spaces required around that operators
Add spaces around operators. Signed-off-by: Zhang Han --- qobject/qdict.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qobject/qdict.c b/qobject/qdict.c index 2c07b3c87f..05ec950e05 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -42,8 +42,8 @@ static unsigned int tdb_hash(const char *name) unsigned i; /* Used to cycle through random values. */ /* Set the initial value from the key size. */ -for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++) -value = (value + (((const unsigned char *)name)[i] << (i*5 % 24))); +for (value = 0x238F13AF * strlen(name), i = 0; name[i]; i++) +value = (value + (((const unsigned char *)name)[i] << (i * 5 % 24))); return (1103515243 * value + 12345); } -- 2.29.1.59.gf9b6481aed
Re: [PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow
On Sat, Dec 19, 2020 at 10:42:24AM +, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland
Looks correct, but it could really do with a rationale in the commit message.
> ---
> hw/ppc/mac_oldworld.c | 30 +++---
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 2ead34bdf1..e58e0525fe 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -227,6 +227,21 @@ static void ppc_heathrow_init(MachineState *machine)
> }
> }
>
> +/* Grackle PCI host bridge */
> +dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> +qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
> +s = SYS_BUS_DEVICE(dev);
> +sysbus_realize_and_unref(s, _fatal);
> +
> +sysbus_mmio_map(s, 0, GRACKLE_BASE);
> +sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
> +/* PCI hole */
> +memory_region_add_subregion(get_system_memory(), 0x8000ULL,
> +sysbus_mmio_get_region(s, 2));
> +/* Register 2 MB of ISA IO space */
> +memory_region_add_subregion(get_system_memory(), 0xfe00,
> +sysbus_mmio_get_region(s, 3));
> +
> /* XXX: we register only 1 output pin for heathrow PIC */
> pic_dev = qdev_new(TYPE_HEATHROW);
> sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), _fatal);
> @@ -251,21 +266,6 @@ static void ppc_heathrow_init(MachineState *machine)
> tbfreq = TBFREQ;
> }
>
> -/* Grackle PCI host bridge */
> -dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> -qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
> -s = SYS_BUS_DEVICE(dev);
> -sysbus_realize_and_unref(s, _fatal);
> -
> -sysbus_mmio_map(s, 0, GRACKLE_BASE);
> -sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
> -/* PCI hole */
> -memory_region_add_subregion(get_system_memory(), 0x8000ULL,
> -sysbus_mmio_get_region(s, 2));
> -/* Register 2 MB of ISA IO space */
> -memory_region_add_subregion(get_system_memory(), 0xfe00,
> -sysbus_mmio_get_region(s, 3));
> -
> for (i = 0; i < 4; i++) {
> qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
> }
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[PATCH 1/4] qobject: open brace '{' following struct go on the same line
Put open brace '{' on the same line of struct.
Signed-off-by: Zhang Han
---
qobject/json-parser.c | 3 +--
qobject/qjson.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index c0f521b56b..18b87a42f3 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -31,8 +31,7 @@ struct JSONToken {
char str[];
};
-typedef struct JSONParserContext
-{
+typedef struct JSONParserContext {
Error *err;
JSONToken *current;
GQueue *buf;
diff --git a/qobject/qjson.c b/qobject/qjson.c
index f1f2c69704..f5623081e3 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -22,8 +22,7 @@
#include "qapi/qmp/qstring.h"
#include "qemu/unicode.h"
-typedef struct JSONParsingState
-{
+typedef struct JSONParsingState {
JSONMessageParser parser;
QObject *result;
Error *err;
--
2.29.1.59.gf9b6481aed
[PATCH 0/4] Fix some style problems in qobject
Some style problems in qobject directory are found by checkpatch.pl. Fix these
style problems.
Zhang Han (4):
qobject: open brace '{' following struct go on the same line
qobject: code indent should never use tabs
qobject: spaces required around that operators
qobject: braces {} are necessary for all arms of this statement
qobject/json-parser.c | 3 +--
qobject/qdict.c | 12 +++-
qobject/qjson.c | 3 +--
3 files changed, 9 insertions(+), 9 deletions(-)
--
2.29.1.59.gf9b6481aed
[PATCH 2/4] qobject: code indent should never use tabs
Transfer tabs to spaces.
Signed-off-by: Zhang Han
---
qobject/qdict.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 1079bd3f6f..2c07b3c87f 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -38,8 +38,8 @@ QDict *qdict_new(void)
*/
static unsigned int tdb_hash(const char *name)
{
-unsigned value;/* Used to compute the hash value. */
-unsigned i; /* Used to cycle through random values. */
+unsigned value;/* Used to compute the hash value. */
+unsigned i; /* Used to cycle through random values. */
/* Set the initial value from the key size. */
for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++)
--
2.29.1.59.gf9b6481aed
Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough
On 2020/12/28 上午8:38, Zhang, Chen wrote:
-Original Message-
From: Jason Wang
Sent: Friday, December 25, 2020 2:20 PM
To: Zhang, Chen ; qemu-dev ; Eric Blake ; Dr. David Alan
Gilbert ; Markus Armbruster
Cc: Zhang Chen
Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
passthrough
On 2020/12/24 上午9:09, Zhang Chen wrote:
From: Zhang Chen
Since the real user scenario does not need to monitor all traffic.
Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
network passthrough list.
Signed-off-by: Zhang Chen
---
net/net.c | 12
qapi/net.json | 46
++
2 files changed, 58 insertions(+)
diff --git a/net/net.c b/net/net.c
index e1035f21d1..eac7a92618 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error
**errp)
qemu_del_net_client(nc);
}
+void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
+ Error **errp) {
+/* Setup passthrough connection */ }
+
+void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
+ Error **errp) {
+/* Delete passthrough connection */ }
+
static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
{
char *str;
diff --git a/qapi/net.json b/qapi/net.json index
c31748c87f..466c29714e 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -714,3 +714,49 @@
##
{ 'event': 'FAILOVER_NEGOTIATED',
'data': {'device-id': 'str'} }
+
+##
+# @colo-passthrough-add:
+#
+# Add passthrough entry according to customer's needs in COLO-compare.
+#
+# @protocol: COLO passthrough just support TCP and UDP.
+#
+# @port: TCP or UDP port number.
+#
+# Returns: Nothing on success
+#
+# Since: 5.3
+#
+# Example:
+#
+# -> { "execute": "colo-passthrough-add",
+# "arguments": { "protocol": "tcp", "port": 3389 } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'colo-passthrough-add',
+ 'data': {'protocol': 'str', 'port': 'uint32'} }
Do we plan to support 4-tuple (src ip,src port, dst ip, dst port) in the
future? If
yes, let's add them now.
And do we plan to support wildcard here?
I think just using the port is enough for COLO compare.
Because in this case, users need passthrough some guest services are
distinguished by static ports.
And for support 4-tuple and wildcard are a good question, do you think we
should add some passthrough
Mechanism for all Qemu net filter? If yes, we should support that in the future.
I think we can start form COLO. To avoid QMP compatibility issues, I
would like to add the n tuple and wildcard support now.
Thanks
Thanks
Chen
Thanks
+
+##
+# @colo-passthrough-del:
+#
+# Delete passthrough entry according to customer's needs in COLO-
compare.
+#
+# @protocol: COLO passthrough just support TCP and UDP.
+#
+# @port: TCP or UDP port number.
+#
+# Returns: Nothing on success
+#
+# Since: 5.3
+#
+# Example:
+#
+# -> { "execute": "colo-passthrough-del",
+# "arguments": { "protocol": "tcp", "port": 3389 } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'colo-passthrough-del',
+ 'data': {'protocol': 'str', 'port': 'uint32'} }
Re: [PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
On Sat, Dec 19, 2020 at 10:42:23AM +, Mark Cave-Ayland wrote:
> This condition will have already been caught when wiring the heathrow PIC
> irqs to the CPU.
>
> Signed-off-by: Mark Cave-Ayland
Reviewed-by: David Gibson
> ---
> hw/ppc/mac_oldworld.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 04f98a4d81..2ead34bdf1 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -251,12 +251,6 @@ static void ppc_heathrow_init(MachineState *machine)
> tbfreq = TBFREQ;
> }
>
> -/* init basic PC hardware */
> -if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
> -error_report("Only 6xx bus is supported on heathrow machine");
> -exit(1);
> -}
> -
> /* Grackle PCI host bridge */
> dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
> qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 4/8] spapr_pci: Fix memory leak of vmstate_spapr_pci
On Sat, Dec 26, 2020 at 06:33:43PM +0800, g00517791 wrote:
> From: Jinhao Gao
>
> When VM migrate VMState of spapr_pci, the field(msi_devs) of spapr_pci
> having a flag of VMS_ALLOC need to allocate memory. If the src doesn't free
> memory of msi_devs in SaveStateEntry of spapr_pci after QEMUFile save
> VMState of spapr_pci, it may result in memory leak of msi_devs. We add the
> post_save func to free memory, which prevents memory leak.
>
> Signed-off-by: Jinhao Gao
Not really a memory leak, since it will get freed on the next
pre_save. But, we might as well free it earlier if we can ,so
Acked-by: David Gibson
> ---
> hw/ppc/spapr_pci.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 76d7c91e9c..1b2b940606 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2173,6 +2173,16 @@ static int spapr_pci_pre_save(void *opaque)
> return 0;
> }
>
> +static int spapr_pci_post_save(void *opaque)
> +{
> +SpaprPhbState *sphb = opaque;
> +
> +g_free(sphb->msi_devs);
> +sphb->msi_devs = NULL;
> +sphb->msi_devs_num = 0;
> +return 0;
> +}
> +
> static int spapr_pci_post_load(void *opaque, int version_id)
> {
> SpaprPhbState *sphb = opaque;
> @@ -2205,6 +2215,7 @@ static const VMStateDescription vmstate_spapr_pci = {
> .version_id = 2,
> .minimum_version_id = 2,
> .pre_save = spapr_pci_pre_save,
> +.post_save = spapr_pci_post_save,
> .post_load = spapr_pci_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT64_EQUAL(buid, SpaprPhbState, NULL),
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH 3/8] spapr: Fix memory leak of vmstate_spapr_event_entry
On Sat, Dec 26, 2020 at 06:33:42PM +0800, g00517791 wrote:
> From: Jinhao Gao
>
> When VM migrate VMState of spapr_event_log_entry, the field(extended_log)
> of spapr_event_log_entry having a flag of VMS_ALLOC needs to allocate
> memory. If the dst doesn't free memory which has been allocated for
> SaveStateEntry of spapr_event_log_entry before dst loads device state,
> it may result that the pointer of extended_log is overlaid when vm loads.
> We add the pre_load func to free memory, which prevents memory leak.
>
> Signed-off-by: Jinhao Gao
Acked-by: David Gibson
> ---
> hw/ppc/spapr.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 489cefcb81..ddfed1e7ca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1799,10 +1799,22 @@ static bool spapr_pending_events_needed(void *opaque)
> return !QTAILQ_EMPTY(>pending_events);
> }
>
> +static int spapr_event_log_entry_pre_load(void *opaque)
> +{
> +SpaprEventLogEntry *entry = opaque;
> +
> +g_free(entry->extended_log);
> +entry->extended_log = NULL;
> +entry->extended_length = 0;
> +
> +return 0;
> +}
> +
> static const VMStateDescription vmstate_spapr_event_entry = {
> .name = "spapr_event_log_entry",
> .version_id = 1,
> .minimum_version_id = 1,
> +.pre_load = spapr_event_log_entry_pre_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT32(summary, SpaprEventLogEntry),
> VMSTATE_UINT32(extended_length, SpaprEventLogEntry),
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [PATCH v2 0/7] fuzz: improve crash case minimization
Patchew URL: https://patchew.org/QEMU/me3p282mb17458b2705c43e860a26171dfc...@me3p282mb1745.ausp282.prod.outlook.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: me3p282mb17458b2705c43e860a26171dfc...@me3p282mb1745.ausp282.prod.outlook.com Subject: [PATCH v2 0/7] fuzz: improve crash case minimization === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/me3p282mb17458b2705c43e860a26171dfc...@me3p282mb1745.ausp282.prod.outlook.com -> patchew/me3p282mb17458b2705c43e860a26171dfc...@me3p282mb1745.ausp282.prod.outlook.com Switched to a new branch 'test' ddad9b9 fuzz: heuristic split write based on past IOs 9ff99f8 fuzz: add minimization options ff015d1 fuzz: set bits in operand of write/out to zero fd57227 fuzz: loop the remove minimizer and refactoring d956f38 fuzz: split write operand using binary approach 06ea622 fuzz: double the IOs to remove for every loop 6e7708d fuzz: accelerate non-crash detection === OUTPUT BEGIN === 1/7 Checking commit 6e7708db32d3 (fuzz: accelerate non-crash detection) ERROR: trailing whitespace #38: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:34: +crash output but indicates the same bug. Under this situation, our minimizer is $ ERROR: trailing whitespace #39: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:35: +incapable of recognizing and stopped from removing it. In the future, we may $ total: 2 errors, 0 warnings, 65 lines checked Patch 1/7 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/7 Checking commit 06ea6221a492 (fuzz: double the IOs to remove for every loop) 3/7 Checking commit d956f387fad9 (fuzz: split write operand using binary approach) ERROR: trailing whitespace #104: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:135: +# it into two separate write commands. If splitting the data operand $ ERROR: trailing whitespace #107: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:138: +# is to prune unneccessary bytes from long writes, while accommodating $ total: 2 errors, 0 warnings, 62 lines checked Patch 3/7 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/7 Checking commit fd57227237c1 (fuzz: loop the remove minimizer and refactoring) ERROR: trailing whitespace #101: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:187: +$ total: 1 errors, 0 warnings, 54 lines checked Patch 4/7 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 5/7 Checking commit ff015d1f3945 (fuzz: set bits in operand of write/out to zero) 6/7 Checking commit 9ff99f86d34b (fuzz: add minimization options) 7/7 Checking commit ddad9b911ad1 (fuzz: heuristic split write based on past IOs) ERROR: trailing whitespace #48: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:113: +$ ERROR: trailing whitespace #54: FILE: scripts/oss-fuzz/minimize_qtest_trace.py:119: +$ total: 2 errors, 0 warnings, 67 lines checked Patch 7/7 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/me3p282mb17458b2705c43e860a26171dfc...@me3p282mb1745.ausp282.prod.outlook.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[PATCH v2 7/7] fuzz: heuristic split write based on past IOs
If previous write commands write the same length of data with the same step,
we view it as a hint.
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 55
1 file changed, 55 insertions(+)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index 7947eb1d40..98bcd0cc8a 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -83,6 +83,42 @@ def check_if_trace_crashes(trace, path):
return False
+# If previous write commands write the same length of data at the same
+# interval, we view it as a hint.
+def split_write_hint(newtrace, i):
+HINT_LEN = 3 # > 2
+if i <=(HINT_LEN-1):
+return None
+
+#find previous continuous write traces
+k = 0
+l = i-1
+writes = []
+while (k != HINT_LEN and l >= 0):
+if newtrace[l].startswith("write "):
+writes.append(newtrace[l])
+k += 1
+l -= 1
+elif newtrace[l] == "":
+l -= 1
+else:
+return None
+if k != HINT_LEN:
+return None
+
+length = int(writes[0].split()[2], 16)
+for j in range(1, HINT_LEN):
+if length != int(writes[j].split()[2], 16):
+return None
+
+step = int(writes[0].split()[1], 16) - int(writes[1].split()[1], 16)
+for j in range(1, HINT_LEN-1):
+if step != int(writes[j].split()[1], 16) - \
+int(writes[j+1].split()[1], 16):
+return None
+
+return (int(writes[0].split()[1], 16)+step, length)
+
def remove_minimizer(newtrace, outpath):
remove_step = 1
@@ -147,6 +183,25 @@ def remove_minimizer(newtrace, outpath):
length = int(newtrace[i].split()[2], 16)
data = newtrace[i].split()[3][2:]
if length > 1:
+
+# Can we get a hint from previous writes?
+hint = split_write_hint(newtrace, i)
+if hint is not None:
+hint_addr = hint[0]
+hint_len = hint[1]
+if hint_addr >= addr and hint_addr+hint_len <= addr+length:
+newtrace[i] = "write {addr} {size} 0x{data}\n".format(
+addr=hex(hint_addr),
+size=hex(hint_len),
+data=data[(hint_addr-addr)*2:\
+(hint_addr-addr)*2+hint_len*2])
+if check_if_trace_crashes(newtrace, outpath):
+# next round
+i += 1
+continue
+newtrace[i] = prior[0]
+
+# Try splitting it using a binary approach
leftlength = int(length/2)
rightlength = length - leftlength
newtrace.insert(i+1, "")
--
2.25.1
[PATCH v2 5/7] fuzz: set bits in operand of write/out to zero
Simplifying the crash cases by opportunistically setting bits in operands of
out/write to zero may help to debug, since usually bit one means turn on or
trigger a function while zero is the default turn-off setting.
Tested Bug 1908062.
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 41 +++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index 0cc88da933..bcb32ee173 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -164,6 +164,42 @@ def remove_minimizer(newtrace, outpath):
i += 1
+def set_zero_minimizer(newtrace, outpath):
+# try setting bits in operands of out/write to zero
+i = 0
+while i < len(newtrace):
+if (not newtrace[i].startswith("write ") and not
+ newtrace[i].startswith("out")):
+ i += 1
+ continue
+# write ADDR SIZE DATA
+# outx ADDR VALUE
+print("\nzero setting bits: {}".format(newtrace[i]))
+
+prefix = " ".join(newtrace[i].split()[:-1])
+data = newtrace[i].split()[-1]
+data_bin = bin(int(data, 16))
+data_bin_list = list(data_bin)
+
+for j in range(2, len(data_bin_list)):
+prior = newtrace[i]
+if (data_bin_list[j] == '1'):
+data_bin_list[j] = '0'
+data_try = hex(int("".join(data_bin_list), 2))
+# It seems qtest only accepts padded hex-values.
+if len(data_try) % 2 == 1:
+data_try = data_try[:2] + "0" + data_try[2:-1]
+
+newtrace[i] = "{prefix} {data_try}\n".format(
+prefix=prefix,
+data_try=data_try)
+
+if not check_if_trace_crashes(newtrace, outpath):
+data_bin_list[j] = '1'
+newtrace[i] = prior
+i += 1
+
+
def minimize_trace(inpath, outpath):
global TIMEOUT
with open(inpath) as f:
@@ -184,7 +220,10 @@ def minimize_trace(inpath, outpath):
old_len = len(newtrace)
remove_minimizer(newtrace, outpath)
newtrace = list(filter(lambda s: s != "", newtrace))
-
+assert(check_if_trace_crashes(newtrace, outpath))
+
+# set zero minimizer
+set_zero_minimizer(newtrace, outpath)
assert(check_if_trace_crashes(newtrace, outpath))
--
2.25.1
[PATCH v2 6/7] fuzz: add minimization options
-M1: loop around the remove minimizer
-M2: try setting bits in operand of write/out to zero
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 30
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index bcb32ee173..7947eb1d40 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -16,6 +16,10 @@ QEMU_PATH = None
TIMEOUT = 5
CRASH_TOKEN = None
+# Minimization levels
+M1 = False # loop around the remove minimizer
+M2 = False # try setting bits in operand of write/out to zero
+
write_suffix_lookup = {"b": (1, "B"),
"w": (2, "H"),
"l": (4, "L"),
@@ -23,10 +27,19 @@ write_suffix_lookup = {"b": (1, "B"),
def usage():
sys.exit("""\
-Usage: QEMU_PATH="/path/to/qemu" QEMU_ARGS="args" {} input_trace output_trace
+Usage:
+
+QEMU_PATH="/path/to/qemu" QEMU_ARGS="args" {} [Options] input_trace
output_trace
+
By default, will try to use the second-to-last line in the output to identify
whether the crash occred. Optionally, manually set a string that idenitifes the
crash by setting CRASH_TOKEN=
+
+Options:
+
+-M1: enable a loop around the remove minimizer, which may help decrease some
+ timing dependant instructions. Off by default.
+-M2: try setting bits in operand of write/out to zero. Off by default.
""".format((sys.argv[0])))
deduplication_note = """\n\
@@ -213,24 +226,31 @@ def minimize_trace(inpath, outpath):
print("Setting the timeout for {} seconds".format(TIMEOUT))
newtrace = trace[:]
-
+global M1, M2
# remove minimizer
old_len = len(newtrace) + 1
while(old_len > len(newtrace)):
old_len = len(newtrace)
+print("trace lenth = ", old_len)
remove_minimizer(newtrace, outpath)
+if not M1 and not M2:
+break
newtrace = list(filter(lambda s: s != "", newtrace))
assert(check_if_trace_crashes(newtrace, outpath))
# set zero minimizer
-set_zero_minimizer(newtrace, outpath)
+if M2:
+set_zero_minimizer(newtrace, outpath)
assert(check_if_trace_crashes(newtrace, outpath))
if __name__ == '__main__':
if len(sys.argv) < 3:
usage()
-
+if "-M1" in sys.argv:
+M1 = True
+if "-M2" in sys.argv:
+M2 = True
QEMU_PATH = os.getenv("QEMU_PATH")
QEMU_ARGS = os.getenv("QEMU_ARGS")
if QEMU_PATH is None or QEMU_ARGS is None:
@@ -239,4 +259,4 @@ if __name__ == '__main__':
# QEMU_ARGS += " -accel qtest"
CRASH_TOKEN = os.getenv("CRASH_TOKEN")
QEMU_ARGS += " -qtest stdio -monitor none -serial none "
-minimize_trace(sys.argv[1], sys.argv[2])
+minimize_trace(sys.argv[-2], sys.argv[-1])
--
2.25.1
[PATCH v2 2/7] fuzz: double the IOs to remove for every loop
Instead of removing IO instructions one by one, we can try deleting multiple
instructions at once. According to the locality of reference, we double the
number of instructions to remove for the next round and recover it to one
once we fail.
This patch is usually significant for large input.
Test with quadrupled trace input at:
https://bugs.launchpad.net/qemu/+bug/1890333/comments/1
Patched 1/6 version:
real 0m45.904s
user 0m16.874s
sys 0m10.042s
Refined version:
real 0m11.412s
user 0m6.888s
sys 0m3.325s
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 33 +++-
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index a290dc0579..71fb0cef32 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -85,19 +85,28 @@ def minimize_trace(inpath, outpath):
i = 0
newtrace = trace[:]
-# For each line
+remove_step = 1
while i < len(newtrace):
-# 1.) Try to remove it completely and reproduce the crash. If it works,
-# we're done.
-prior = newtrace[i]
-print("Trying to remove {}".format(newtrace[i]))
-# Try to remove the line completely
-newtrace[i] = ""
+# 1.) Try to remove lines completely and reproduce the crash.
+# If it works, we're done.
+if (i+remove_step) >= len(newtrace):
+remove_step = 1
+prior = newtrace[i:i+remove_step]
+for j in range(i, i+remove_step):
+newtrace[j] = ""
+print("Removing {lines} ...".format(lines=prior))
if check_if_trace_crashes(newtrace, outpath):
-i += 1
+i += remove_step
+# Double the number of lines to remove for next round
+remove_step *= 2
continue
-newtrace[i] = prior
-
+# Failed to remove multiple IOs, fast recovery
+if remove_step > 1:
+for j in range(i, i+remove_step):
+newtrace[j] = prior[j-i]
+remove_step = 1
+continue
+newtrace[i] = prior[0] # remove_step = 1
# 2.) Try to replace write{bwlq} commands with a write addr, len
# command. Since this can require swapping endianness, try both LE and
# BE options. We do this, so we can "trim" the writes in (3)
@@ -118,7 +127,7 @@ def minimize_trace(inpath, outpath):
if(check_if_trace_crashes(newtrace, outpath)):
break
else:
-newtrace[i] = prior
+newtrace[i] = prior[0]
# 3.) If it is a qtest write command: write addr len data, try to split
# it into two separate write commands. If splitting the write down the
@@ -151,7 +160,7 @@ def minimize_trace(inpath, outpath):
if check_if_trace_crashes(newtrace, outpath):
i -= 1
else:
-newtrace[i] = prior
+newtrace[i] = prior[0]
del newtrace[i+1]
i += 1
check_if_trace_crashes(newtrace, outpath)
--
2.25.1
[PATCH v2 4/7] fuzz: loop the remove minimizer and refactoring
Now we use a one-time scan and remove strategy in the remval minimizer,
which is not suitable for timing dependent instructions.
For example, instruction A will indicate an address where the config
chunk locates, and instruction B will make the configuration active. If
we have the following instruction sequence:
...
A1
B1
A2
B2
...
A2 and B2 are the actual instructions that trigger the bug.
If we scan from top to bottom, after we remove A1, the behavior of B1
might be unknowable, including not to crash the program. But we will
successfully remove B1 later cause A2 and B2 will crash the process
anyway:
...
A1
A2
B2
...
Now one more trimming will remove A1.
In the perfect case, we would need to be able to remove A and B (or C!) at
the same time. But for now, let's just add a loop around the minimizer.
Since we only remove instructions, this iterative algorithm is converging.
Tested with Bug 1908062.
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 41 +++-
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index dd6eeb7258..0cc88da933 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -71,21 +71,9 @@ def check_if_trace_crashes(trace, path):
return False
-def minimize_trace(inpath, outpath):
-global TIMEOUT
-with open(inpath) as f:
-trace = f.readlines()
-start = time.time()
-if not check_if_trace_crashes(trace, outpath):
-sys.exit("The input qtest trace didn't cause a crash...")
-end = time.time()
-print("Crashed in {} seconds".format(end-start))
-TIMEOUT = (end-start)*5
-print("Setting the timeout for {} seconds".format(TIMEOUT))
-
-i = 0
-newtrace = trace[:]
+def remove_minimizer(newtrace, outpath):
remove_step = 1
+i = 0
while i < len(newtrace):
# 1.) Try to remove lines completely and reproduce the crash.
# If it works, we're done.
@@ -174,7 +162,30 @@ def minimize_trace(inpath, outpath):
newtrace[i] = prior[0]
del newtrace[i+1]
i += 1
-check_if_trace_crashes(newtrace, outpath)
+
+
+def minimize_trace(inpath, outpath):
+global TIMEOUT
+with open(inpath) as f:
+trace = f.readlines()
+start = time.time()
+if not check_if_trace_crashes(trace, outpath):
+sys.exit("The input qtest trace didn't cause a crash...")
+end = time.time()
+print("Crashed in {} seconds".format(end-start))
+TIMEOUT = (end-start)*5
+print("Setting the timeout for {} seconds".format(TIMEOUT))
+
+newtrace = trace[:]
+
+# remove minimizer
+old_len = len(newtrace) + 1
+while(old_len > len(newtrace)):
+old_len = len(newtrace)
+remove_minimizer(newtrace, outpath)
+newtrace = list(filter(lambda s: s != "", newtrace))
+
+assert(check_if_trace_crashes(newtrace, outpath))
if __name__ == '__main__':
--
2.25.1
[PATCH v2 3/7] fuzz: split write operand using binary approach
Currently, we split the write commands' data from the middle. If it does not
work, try to move the pivot left by one byte and retry until there is no
space.
But, this method has two flaws:
1. It may fail to trim all unnecessary bytes on the right side.
For example, there is an IO write command:
write addr uuuu
u is the unnecessary byte for the crash. Unlike ram write commands, in most
case, a split IO write won't trigger the same crash, So if we split from the
middle, we will get:
write addr uu (will be removed in next round)
write addr uu
For uu, since split it from the middle and retry to the leftmost byte
won't get the same crash, we will be stopped from removing the last two
bytes.
2. The algorithm complexity is O(n) since we move the pivot byte by byte.
To solve the first issue, we can try a symmetrical position on the right if
we fail on the left. As for the second issue, instead moving by one byte, we
can approach the boundary exponentially, achieving O(log(n)).
Give an example:
uu len=6
+
|
+
xxx,xuu 6/2=3 fail
+
+--+-+
||
++
xx,xxuu 6/2^2=1 fail u,u 6-1=5 success
+ +
+--++ |
| |+-+ u removed
+ +
xx,xxu 5/2=2 fail ,u 6-2=4 success
+
|
+---+ u removed
In some rare case, this algorithm will fail to trim all unnecessary bytes:
xuxx
-xuxx Fail
-xuxx Fail
xuxx- Fail
...
I think the trade-off is worth it.
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 29
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index 71fb0cef32..dd6eeb7258 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -94,7 +94,7 @@ def minimize_trace(inpath, outpath):
prior = newtrace[i:i+remove_step]
for j in range(i, i+remove_step):
newtrace[j] = ""
-print("Removing {lines} ...".format(lines=prior))
+print("Removing {lines} ...\n".format(lines=prior))
if check_if_trace_crashes(newtrace, outpath):
i += remove_step
# Double the number of lines to remove for next round
@@ -107,9 +107,11 @@ def minimize_trace(inpath, outpath):
remove_step = 1
continue
newtrace[i] = prior[0] # remove_step = 1
+
# 2.) Try to replace write{bwlq} commands with a write addr, len
# command. Since this can require swapping endianness, try both LE and
# BE options. We do this, so we can "trim" the writes in (3)
+
if (newtrace[i].startswith("write") and not
newtrace[i].startswith("write ")):
suffix = newtrace[i].split()[0][-1]
@@ -130,11 +132,15 @@ def minimize_trace(inpath, outpath):
newtrace[i] = prior[0]
# 3.) If it is a qtest write command: write addr len data, try to split
-# it into two separate write commands. If splitting the write down the
-# middle does not work, try to move the pivot "left" and retry, until
-# there is no space left. The idea is to prune unneccessary bytes from
-# long writes, while accommodating arbitrary MemoryRegion access sizes
-# and alignments.
+# it into two separate write commands. If splitting the data operand
+# from length/2^n bytes to the left does not work, try to move the
pivot
+# to the right side, then add one to n, until length/2^n == 0. The idea
+# is to prune unneccessary bytes from long writes, while accommodating
+# arbitrary MemoryRegion access sizes and alignments.
+
+# This algorithm will fail under some rare situations.
+# e.g., xuxx (u is the unnecessary byte)
+
if newtrace[i].startswith("write "):
addr = int(newtrace[i].split()[1], 16)
length = int(newtrace[i].split()[2], 16)
@@ -143,6 +149,7 @@ def minimize_trace(inpath, outpath):
leftlength = int(length/2)
rightlength = length - leftlength
newtrace.insert(i+1, "")
+power = 1
while leftlength > 0:
newtrace[i] = "write {addr} {size} 0x{data}\n".format(
addr=hex(addr),
@@ -154,9 +161,13 @@ def minimize_trace(inpath, outpath):
data=data[leftlength*2:])
if
[PATCH v2 1/7] fuzz: accelerate non-crash detection
We spend much time waiting for the timeout program during the minimization
process until it passes a time limit. This patch hacks the CLOSED (indicates
the redirection file closed) notification in QTest's output if it doesn't
crash.
Test with quadrupled trace input at:
https://bugs.launchpad.net/qemu/+bug/1890333/comments/1
Original version:
real 1m37.246s
user 0m13.069s
sys 0m8.399s
Refined version:
real 0m45.904s
user 0m16.874s
sys 0m10.042s
Signed-off-by: Qiuhao Li
---
scripts/oss-fuzz/minimize_qtest_trace.py | 41
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
b/scripts/oss-fuzz/minimize_qtest_trace.py
index 5e405a0d5f..a290dc0579 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -29,30 +29,46 @@ whether the crash occred. Optionally, manually set a string
that idenitifes the
crash by setting CRASH_TOKEN=
""".format((sys.argv[0])))
+deduplication_note = """\n\
+Note: While trimming the input, sometimes the mutated trace triggers a
different
+crash output but indicates the same bug. Under this situation, our minimizer
is
+incapable of recognizing and stopped from removing it. In the future, we may
+use a more sophisticated crash case deduplication method.
+\n"""
+
def check_if_trace_crashes(trace, path):
-global CRASH_TOKEN
with open(path, "w") as tracefile:
tracefile.write("".join(trace))
-rc = subprocess.Popen("timeout -s 9 {timeout}s {qemu_path} {qemu_args}
2>&1\
+proc = subprocess.Popen("timeout {timeout}s {qemu_path} {qemu_args} 2>&1\
< {trace_path}".format(timeout=TIMEOUT,
qemu_path=QEMU_PATH,
qemu_args=QEMU_ARGS,
trace_path=path),
shell=True,
stdin=subprocess.PIPE,
- stdout=subprocess.PIPE)
-stdo = rc.communicate()[0]
-output = stdo.decode('unicode_escape')
-if rc.returncode == 137:# Timed Out
-return False
-if len(output.splitlines()) < 2:
-return False
-
+ stdout=subprocess.PIPE,
+ encoding="utf-8")
+global CRASH_TOKEN
if CRASH_TOKEN is None:
-CRASH_TOKEN = output.splitlines()[-2]
+try:
+outs, _ = proc.communicate(timeout=5)
+CRASH_TOKEN = outs.splitlines()[-2]
+except subprocess.TimeoutExpired:
+print("subprocess.TimeoutExpired")
+return False
+print("Identifying Crashes by this string: {}".format(CRASH_TOKEN))
+global deduplication_note
+print(deduplication_note)
+return True
-return CRASH_TOKEN in output
+for line in iter(proc.stdout.readline, b''):
+if "CLOSED" in line:
+return False
+if CRASH_TOKEN in line:
+return True
+
+return False
def minimize_trace(inpath, outpath):
@@ -66,7 +82,6 @@ def minimize_trace(inpath, outpath):
print("Crashed in {} seconds".format(end-start))
TIMEOUT = (end-start)*5
print("Setting the timeout for {} seconds".format(TIMEOUT))
-print("Identifying Crashes by this string: {}".format(CRASH_TOKEN))
i = 0
newtrace = trace[:]
--
2.25.1
[PATCH v2 0/7] fuzz: improve crash case minimization
Extend and refine the crash case minimization process. Test input: Bug 1909261 full_reproducer 6500 QTest instructions (write mostly) Refined (-M1 minimization level) vs. Original version: real 38m31.942s <-- real 532m57.192s user 28m18.188s <-- user 89m0.536s sys 12m42.239s <-- sys 50m33.074s 2558 instructions <-- 2846 instructions Test Enviroment: i7-8550U, 16GB LPDDR3, SSD Ubuntu 20.04.1 5.4.0-58-generic x86_64 Python 3.8.5 v1 --> v2: New: [PATCH v2 1/7] New: [PATCH v2 2/7] New: [PATCH v2 4/7] New: [PATCH v2 6/7] New: [PATCH v2 7/7] Fix: [PATCH 2/4] split using binary approach Fix: [PATCH 3/4] typo in comments Discard: [PATCH 1/4] the hardcoded regex match for crash detection Discard: [PATCH 4/4] the delaying minimizer Thanks for the suggestions from: Alexander Bulekov Qiuhao Li (7): fuzz: accelerate non-crash detection fuzz: double the IOs to remove for every loop fuzz: split write operand using binary approach fuzz: loop the remove minimizer and refactoring fuzz: set bits in operand of write/out to zero fuzz: add minimization options fuzz: heuristic split write based on past IOs scripts/oss-fuzz/minimize_qtest_trace.py | 256 ++- 1 file changed, 208 insertions(+), 48 deletions(-) -- 2.25.1
[PATCH v2 09/10] vt82c686: Convert debug printf to trace points
Drop DPRINTF and use trace functions instead. Two debug messages about
unimplemented registers could be converted to qemu_log_mask() but in
reality all registers are currently unimplemented (we just store and
return values of writable regs but do nothing with them). As we
already trace register access there's no need for additional debug
messages so these are just removed and a comment is added as a reminder.
Signed-off-by: BALATON Zoltan
---
v2: Extended commit message
hw/isa/trace-events | 6 ++
hw/isa/vt82c686.c | 51 +
2 files changed, 21 insertions(+), 36 deletions(-)
diff --git a/hw/isa/trace-events b/hw/isa/trace-events
index 3544c6213c..d267d3e652 100644
--- a/hw/isa/trace-events
+++ b/hw/isa/trace-events
@@ -13,3 +13,9 @@ pc87312_io_write(uint32_t addr, uint32_t val) "write
addr=0x%x val=0x%x"
# apm.c
apm_io_read(uint8_t addr, uint8_t val) "read addr=0x%x val=0x%02x"
apm_io_write(uint8_t addr, uint8_t val) "write addr=0x%x val=0x%02x"
+
+# vt82c686.c
+via_isa_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len
0x%x"
+via_pm_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len
0x%x"
+via_superio_read(uint8_t addr, uint8_t val) "addr 0x%x val 0x%x"
+via_superio_write(uint8_t addr, uint32_t val) "addr 0x%x val 0x%x"
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index cd87ec0103..d7ce15bf9f 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -27,14 +27,7 @@
#include "qemu/timer.h"
#include "exec/address-spaces.h"
#include "qom/object.h"
-
-/* #define DEBUG_VT82C686B */
-
-#ifdef DEBUG_VT82C686B
-#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__, ##__VA_ARGS__)
-#else
-#define DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
typedef struct SuperIOConfig {
uint8_t config[0x100];
@@ -55,12 +48,12 @@ static void superio_ioport_writeb(void *opaque, hwaddr
addr, uint64_t data,
{
SuperIOConfig *superio_conf = opaque;
-DPRINTF("superio_ioport_writeb address 0x%x val 0x%x\n", addr, data);
-if (addr == 0x3f0) {
+if (addr == 0x3f0) { /* config index register */
superio_conf->index = data & 0xff;
} else {
bool can_write = true;
-/* 0x3f1 */
+/* 0x3f1, config data register */
+trace_via_superio_write(superio_conf->index, data & 0xff);
switch (superio_conf->index) {
case 0x00 ... 0xdf:
case 0xe4:
@@ -73,18 +66,7 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr,
uint64_t data,
case 0xfd ... 0xff:
can_write = false;
break;
-case 0xe7:
-if ((data & 0xff) != 0xfe) {
-DPRINTF("change uart 1 base. unsupported yet\n");
-can_write = false;
-}
-break;
-case 0xe8:
-if ((data & 0xff) != 0xbe) {
-DPRINTF("change uart 2 base. unsupported yet\n");
-can_write = false;
-}
-break;
+/* case 0xe6 ... 0xe8: Should set base port of parallel and serial */
default:
break;
@@ -98,9 +80,10 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr,
uint64_t data,
static uint64_t superio_ioport_readb(void *opaque, hwaddr addr, unsigned size)
{
SuperIOConfig *superio_conf = opaque;
+uint8_t val = superio_conf->config[superio_conf->index];
-DPRINTF("superio_ioport_readb address 0x%x\n", addr);
-return superio_conf->config[superio_conf->index];
+trace_via_superio_read(superio_conf->index, val);
+return val;
}
static const MemoryRegionOps superio_ops = {
@@ -141,16 +124,14 @@ static void vt82c686b_isa_reset(DeviceState *dev)
}
/* write config pci function0 registers. PCI-ISA bridge */
-static void vt82c686b_write_config(PCIDevice *d, uint32_t address,
+static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
uint32_t val, int len)
{
VT82C686BISAState *vt686 = VT82C686B_ISA(d);
-DPRINTF("vt82c686b_write_config address 0x%x val 0x%x len 0x%x\n",
- address, val, len);
-
-pci_default_write_config(d, address, val, len);
-if (address == 0x85) { /* enable or disable super IO configure */
+trace_via_isa_write(addr, val, len);
+pci_default_write_config(d, addr, val, len);
+if (addr == 0x85) { /* enable or disable super IO configure */
memory_region_set_enabled(>superio, val & 0x2);
}
}
@@ -203,12 +184,10 @@ static void pm_io_space_update(VT686PMState *s)
memory_region_transaction_commit();
}
-static void pm_write_config(PCIDevice *d,
-uint32_t address, uint32_t val, int len)
+static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
{
-DPRINTF("pm_write_config address 0x%x val 0x%x len 0x%x\n",
- address, val, len);
-pci_default_write_config(d, address, val, len);
+
[PATCH v2 08/10] vt82c686: Remove legacy vt82c686b_pm_init() function
Remove legacy vt82c686b_pm_init() function and also rename
VT82C686B_PM type name to match other device names.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
v2: Reworded commit message, delete i2c include here
hw/isa/vt82c686.c | 18 --
hw/mips/fuloong2e.c | 5 -
include/hw/isa/vt82c686.h | 5 +
3 files changed, 5 insertions(+), 23 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 2912c253dc..cd87ec0103 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -12,7 +12,6 @@
#include "qemu/osdep.h"
#include "hw/isa/vt82c686.h"
-#include "hw/i2c/i2c.h"
#include "hw/pci/pci.h"
#include "hw/qdev-properties.h"
#include "hw/isa/isa.h"
@@ -167,7 +166,6 @@ struct VT686PMState {
uint32_t smb_io_base;
};
-#define TYPE_VT82C686B_PM "VT82C686B_PM"
OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
static void pm_update_sci(VT686PMState *s)
@@ -271,22 +269,6 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error
**errp)
acpi_pm1_cnt_init(>ar, >io, false, false, 2);
}
-I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq sci_irq)
-{
-PCIDevice *dev;
-VT686PMState *s;
-
-dev = pci_new(devfn, TYPE_VT82C686B_PM);
-qdev_prop_set_uint32(>qdev, "smb_io_base", smb_io_base);
-
-s = VT82C686B_PM(dev);
-
-pci_realize_and_unref(dev, bus, _fatal);
-
-return s->smb.smbus;
-}
-
static Property via_pm_properties[] = {
DEFINE_PROP_UINT32("smb_io_base", VT686PMState, smb_io_base, 0),
DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index d123e34d9e..a2b69a3a7a 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -263,7 +263,10 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus,
int slot, qemu_irq intc,
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
-*i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1, NULL);
+dev = pci_new(PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
+qdev_prop_set_uint32(DEVICE(dev), "smb_io_base", 0xeee1);
+pci_realize_and_unref(dev, pci_bus, _fatal);
+*i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
/* Audio support */
pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 8d2d276fe1..5b0a1ffe72 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -3,11 +3,8 @@
#define TYPE_VT82C686B_ISA "vt82c686b-isa"
#define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
+#define TYPE_VT82C686B_PM "vt82c686b-pm"
#define TYPE_VIA_AC97 "via-ac97"
#define TYPE_VIA_MC97 "via-mc97"
-/* vt82c686.c */
-I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
- qemu_irq sci_irq);
-
#endif
--
2.21.3
[PATCH v2 06/10] audio/via-ac97: Simplify code and set user_creatable to false
Remove some unneded, empty code and set user_creatable to false
(besides being not implemented yet, so does nothing anyway) it's also
normally part of VIA south bridge chips so no need to confuse users
showing them these devices.
Signed-off-by: BALATON Zoltan
---
hw/audio/via-ac97.c | 51 +
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/hw/audio/via-ac97.c b/hw/audio/via-ac97.c
index e617416ff7..6d556f74fc 100644
--- a/hw/audio/via-ac97.c
+++ b/hw/audio/via-ac97.c
@@ -13,27 +13,13 @@
#include "hw/isa/vt82c686.h"
#include "hw/pci/pci.h"
-struct VIAAC97State {
-PCIDevice dev;
-};
-
-struct VIAMC97State {
-PCIDevice dev;
-};
-
-OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97)
-OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97)
-
-static void via_ac97_realize(PCIDevice *dev, Error **errp)
+static void via_ac97_realize(PCIDevice *pci_dev, Error **errp)
{
-VIAAC97State *s = VIA_AC97(dev);
-uint8_t *pci_conf = s->dev.config;
-
-pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
- PCI_COMMAND_PARITY);
-pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_CAP_LIST |
- PCI_STATUS_DEVSEL_MEDIUM);
-pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
+pci_set_word(pci_dev->config + PCI_COMMAND,
+ PCI_COMMAND_INVALIDATE | PCI_COMMAND_PARITY);
+pci_set_word(pci_dev->config + PCI_STATUS,
+ PCI_STATUS_CAP_LIST | PCI_STATUS_DEVSEL_MEDIUM);
+pci_set_long(pci_dev->config + PCI_INTERRUPT_PIN, 0x03);
}
static void via_ac97_class_init(ObjectClass *klass, void *data)
@@ -47,13 +33,15 @@ static void via_ac97_class_init(ObjectClass *klass, void
*data)
k->revision = 0x50;
k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
-dc->desc = "AC97";
+dc->desc = "VIA AC97";
+/* Reason: Part of a south bridge chip */
+dc->user_creatable = false;
}
static const TypeInfo via_ac97_info = {
.name = TYPE_VIA_AC97,
.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VIAAC97State),
+.instance_size = sizeof(PCIDevice),
.class_init= via_ac97_class_init,
.interfaces = (InterfaceInfo[]) {
{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
@@ -61,15 +49,12 @@ static const TypeInfo via_ac97_info = {
},
};
-static void via_mc97_realize(PCIDevice *dev, Error **errp)
+static void via_mc97_realize(PCIDevice *pci_dev, Error **errp)
{
-VIAMC97State *s = VIA_MC97(dev);
-uint8_t *pci_conf = s->dev.config;
-
-pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
- PCI_COMMAND_VGA_PALETTE);
-pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
-pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
+pci_set_word(pci_dev->config + PCI_COMMAND,
+ PCI_COMMAND_INVALIDATE | PCI_COMMAND_VGA_PALETTE);
+pci_set_word(pci_dev->config + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
+pci_set_long(pci_dev->config + PCI_INTERRUPT_PIN, 0x03);
}
static void via_mc97_class_init(ObjectClass *klass, void *data)
@@ -83,13 +68,15 @@ static void via_mc97_class_init(ObjectClass *klass, void
*data)
k->class_id = PCI_CLASS_COMMUNICATION_OTHER;
k->revision = 0x30;
set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
-dc->desc = "MC97";
+dc->desc = "VIA MC97";
+/* Reason: Part of a south bridge chip */
+dc->user_creatable = false;
}
static const TypeInfo via_mc97_info = {
.name = TYPE_VIA_MC97,
.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VIAMC97State),
+.instance_size = sizeof(PCIDevice),
.class_init= via_mc97_class_init,
.interfaces = (InterfaceInfo[]) {
{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
--
2.21.3
[PATCH v2 07/10] vt82c686: Remove legacy vt82c686b_isa_init() function
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
v2: Reworded commit message
hw/isa/vt82c686.c | 9 -
hw/mips/fuloong2e.c | 4 +++-
include/hw/isa/vt82c686.h | 3 +--
3 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9567326d8e..2912c253dc 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -49,7 +49,6 @@ struct VT82C686BISAState {
SuperIOConfig superio_conf;
};
-#define TYPE_VT82C686B_ISA "vt82c686b-isa"
OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
@@ -367,14 +366,6 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
>superio);
}
-ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
-{
-PCIDevice *d;
-
-d = pci_create_simple_multifunction(bus, devfn, true, TYPE_VT82C686B_ISA);
-return ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
-}
-
static void via_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 3b0489f781..d123e34d9e 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -240,7 +240,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
ISABus *isa_bus;
PCIDevice *dev;
-isa_bus = vt82c686b_isa_init(pci_bus, PCI_DEVFN(slot, 0));
+dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
+ TYPE_VT82C686B_ISA);
+isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(dev), "isa.0"));
assert(isa_bus);
*p_isa_bus = isa_bus;
/* Interrupt controller */
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index ff80a926dc..8d2d276fe1 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -1,13 +1,12 @@
#ifndef HW_VT82C686_H
#define HW_VT82C686_H
-
+#define TYPE_VT82C686B_ISA "vt82c686b-isa"
#define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
#define TYPE_VIA_AC97 "via-ac97"
#define TYPE_VIA_MC97 "via-mc97"
/* vt82c686.c */
-ISABus *vt82c686b_isa_init(PCIBus * bus, int devfn);
I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
qemu_irq sci_irq);
--
2.21.3
[PATCH v2 10/10] vt82c686: Remove unneeded includes and defines
These are not used or not needed.
Signed-off-by: BALATON Zoltan
---
v2: Added back a few that we get indirectly but keep it explicit
hw/isa/vt82c686.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d7ce15bf9f..02d6759c00 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -16,9 +16,7 @@
#include "hw/qdev-properties.h"
#include "hw/isa/isa.h"
#include "hw/isa/superio.h"
-#include "hw/sysbus.h"
#include "migration/vmstate.h"
-#include "hw/mips/mips.h"
#include "hw/isa/apm.h"
#include "hw/acpi/acpi.h"
#include "hw/i2c/pm_smbus.h"
@@ -26,7 +24,6 @@
#include "qemu/module.h"
#include "qemu/timer.h"
#include "exec/address-spaces.h"
-#include "qom/object.h"
#include "trace.h"
typedef struct SuperIOConfig {
@@ -136,8 +133,6 @@ static void vt82c686b_write_config(PCIDevice *d, uint32_t
addr,
}
}
-#define ACPI_DBG_IO_ADDR 0xb044
-
struct VT686PMState {
PCIDevice dev;
MemoryRegion io;
--
2.21.3
[PATCH v2 05/10] vt82c686: Split off via-[am]c97 into separate file in hw/audio
The via-[am]c97 code is supposed to implement the audio part of VIA
south bridge chips so it is better placed under hw/audio/. Split it
off into a separate file.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
v2: Reworded commit message
hw/audio/meson.build | 1 +
hw/audio/via-ac97.c | 106 +++
hw/isa/vt82c686.c| 91 -
3 files changed, 107 insertions(+), 91 deletions(-)
create mode 100644 hw/audio/via-ac97.c
diff --git a/hw/audio/meson.build b/hw/audio/meson.build
index 549e9a0396..32c42bdebe 100644
--- a/hw/audio/meson.build
+++ b/hw/audio/meson.build
@@ -11,4 +11,5 @@ softmmu_ss.add(when: 'CONFIG_MILKYMIST', if_true:
files('milkymist-ac97.c'))
softmmu_ss.add(when: 'CONFIG_PCSPK', if_true: files('pcspk.c'))
softmmu_ss.add(when: 'CONFIG_PL041', if_true: files('pl041.c', 'lm4549.c'))
softmmu_ss.add(when: 'CONFIG_SB16', if_true: files('sb16.c'))
+softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('via-ac97.c'))
softmmu_ss.add(when: 'CONFIG_WM8750', if_true: files('wm8750.c'))
diff --git a/hw/audio/via-ac97.c b/hw/audio/via-ac97.c
new file mode 100644
index 00..e617416ff7
--- /dev/null
+++ b/hw/audio/via-ac97.c
@@ -0,0 +1,106 @@
+/*
+ * VIA south bridges sound support
+ *
+ * This work is licensed under the GNU GPL license version 2 or later.
+ */
+
+/*
+ * TODO: This is entirely boiler plate just registering empty PCI devices
+ * with the right ID guests expect, functionality should be added here.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/isa/vt82c686.h"
+#include "hw/pci/pci.h"
+
+struct VIAAC97State {
+PCIDevice dev;
+};
+
+struct VIAMC97State {
+PCIDevice dev;
+};
+
+OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97)
+OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97)
+
+static void via_ac97_realize(PCIDevice *dev, Error **errp)
+{
+VIAAC97State *s = VIA_AC97(dev);
+uint8_t *pci_conf = s->dev.config;
+
+pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
+ PCI_COMMAND_PARITY);
+pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_CAP_LIST |
+ PCI_STATUS_DEVSEL_MEDIUM);
+pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
+}
+
+static void via_ac97_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+k->realize = via_ac97_realize;
+k->vendor_id = PCI_VENDOR_ID_VIA;
+k->device_id = PCI_DEVICE_ID_VIA_AC97;
+k->revision = 0x50;
+k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
+set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
+dc->desc = "AC97";
+}
+
+static const TypeInfo via_ac97_info = {
+.name = TYPE_VIA_AC97,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(VIAAC97State),
+.class_init= via_ac97_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ },
+},
+};
+
+static void via_mc97_realize(PCIDevice *dev, Error **errp)
+{
+VIAMC97State *s = VIA_MC97(dev);
+uint8_t *pci_conf = s->dev.config;
+
+pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
+ PCI_COMMAND_VGA_PALETTE);
+pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM);
+pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
+}
+
+static void via_mc97_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+k->realize = via_mc97_realize;
+k->vendor_id = PCI_VENDOR_ID_VIA;
+k->device_id = PCI_DEVICE_ID_VIA_MC97;
+k->class_id = PCI_CLASS_COMMUNICATION_OTHER;
+k->revision = 0x30;
+set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
+dc->desc = "MC97";
+}
+
+static const TypeInfo via_mc97_info = {
+.name = TYPE_VIA_MC97,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(VIAMC97State),
+.class_init= via_mc97_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
+{ },
+},
+};
+
+static void via_ac97_register_types(void)
+{
+type_register_static(_ac97_info);
+type_register_static(_mc97_info);
+}
+
+type_init(via_ac97_register_types)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8677a2d212..9567326d8e 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -168,14 +168,6 @@ struct VT686PMState {
uint32_t smb_io_base;
};
-struct VIAAC97State {
-PCIDevice dev;
-};
-
-struct VIAMC97State {
-PCIDevice dev;
-};
-
#define TYPE_VT82C686B_PM "VT82C686B_PM"
OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
@@ -247,87 +239,6 @@ static const VMStateDescription vmstate_acpi = {
}
};
-/*
- * TODO: VIA_AC97 and VIA_MC97
- * just register a PCI device now, functionalities will be implemented later.
- */
-
-OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97)
[PATCH v2 02/10] vt82c686: Remove unnecessary _DEVICE suffix from type macros
There's no reason to suffix everything with _DEVICE when the names are
already unique without it and shorter names are more readable.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/vt82c686.c | 48 +++
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 2a0f85dea9..1be1169f83 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -49,8 +49,8 @@ struct VT82C686BState {
SuperIOConfig superio_conf;
};
-#define TYPE_VT82C686B_DEVICE "VT82C686B"
-OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B_DEVICE)
+#define TYPE_VT82C686B "VT82C686B"
+OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B)
static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
unsigned size)
@@ -117,7 +117,7 @@ static const MemoryRegionOps superio_ops = {
static void vt82c686b_isa_reset(DeviceState *dev)
{
-VT82C686BState *vt82c = VT82C686B_DEVICE(dev);
+VT82C686BState *vt82c = VT82C686B(dev);
uint8_t *pci_conf = vt82c->dev.config;
pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
@@ -146,7 +146,7 @@ static void vt82c686b_isa_reset(DeviceState *dev)
static void vt82c686b_write_config(PCIDevice *d, uint32_t address,
uint32_t val, int len)
{
-VT82C686BState *vt686 = VT82C686B_DEVICE(d);
+VT82C686BState *vt686 = VT82C686B(d);
DPRINTF("vt82c686b_write_config address 0x%x val 0x%x len 0x%x\n",
address, val, len);
@@ -176,14 +176,14 @@ struct VIAMC97State {
PCIDevice dev;
};
-#define TYPE_VT82C686B_PM_DEVICE "VT82C686B_PM"
-OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM_DEVICE)
+#define TYPE_VT82C686B_PM "VT82C686B_PM"
+OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
-#define TYPE_VIA_MC97_DEVICE "VIA_MC97"
-OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97_DEVICE)
+#define TYPE_VIA_MC97 "VIA_MC97"
+OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97)
-#define TYPE_VIA_AC97_DEVICE "VIA_AC97"
-OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97_DEVICE)
+#define TYPE_VIA_AC97 "VIA_AC97"
+OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97)
static void pm_update_sci(VT686PMState *s)
{
@@ -260,7 +260,7 @@ static const VMStateDescription vmstate_acpi = {
static void vt82c686b_ac97_realize(PCIDevice *dev, Error **errp)
{
-VIAAC97State *s = VIA_AC97_DEVICE(dev);
+VIAAC97State *s = VIA_AC97(dev);
uint8_t *pci_conf = s->dev.config;
pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
@@ -274,7 +274,7 @@ void vt82c686b_ac97_init(PCIBus *bus, int devfn)
{
PCIDevice *dev;
-dev = pci_new(devfn, TYPE_VIA_AC97_DEVICE);
+dev = pci_new(devfn, TYPE_VIA_AC97);
pci_realize_and_unref(dev, bus, _fatal);
}
@@ -293,7 +293,7 @@ static void via_ac97_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo via_ac97_info = {
-.name = TYPE_VIA_AC97_DEVICE,
+.name = TYPE_VIA_AC97,
.parent= TYPE_PCI_DEVICE,
.instance_size = sizeof(VIAAC97State),
.class_init= via_ac97_class_init,
@@ -305,7 +305,7 @@ static const TypeInfo via_ac97_info = {
static void vt82c686b_mc97_realize(PCIDevice *dev, Error **errp)
{
-VIAMC97State *s = VIA_MC97_DEVICE(dev);
+VIAMC97State *s = VIA_MC97(dev);
uint8_t *pci_conf = s->dev.config;
pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
@@ -318,7 +318,7 @@ void vt82c686b_mc97_init(PCIBus *bus, int devfn)
{
PCIDevice *dev;
-dev = pci_new(devfn, TYPE_VIA_MC97_DEVICE);
+dev = pci_new(devfn, TYPE_VIA_MC97);
pci_realize_and_unref(dev, bus, _fatal);
}
@@ -337,7 +337,7 @@ static void via_mc97_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo via_mc97_info = {
-.name = TYPE_VIA_MC97_DEVICE,
+.name = TYPE_VIA_MC97,
.parent= TYPE_PCI_DEVICE,
.instance_size = sizeof(VIAMC97State),
.class_init= via_mc97_class_init,
@@ -350,7 +350,7 @@ static const TypeInfo via_mc97_info = {
/* vt82c686 pm init */
static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
{
-VT686PMState *s = VT82C686B_PM_DEVICE(dev);
+VT686PMState *s = VT82C686B_PM(dev);
uint8_t *pci_conf;
pci_conf = s->dev.config;
@@ -386,10 +386,10 @@ I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn,
uint32_t smb_io_base,
PCIDevice *dev;
VT686PMState *s;
-dev = pci_new(devfn, TYPE_VT82C686B_PM_DEVICE);
+dev = pci_new(devfn, TYPE_VT82C686B_PM);
qdev_prop_set_uint32(>qdev, "smb_io_base", smb_io_base);
-s = VT82C686B_PM_DEVICE(dev);
+s = VT82C686B_PM(dev);
pci_realize_and_unref(dev, bus, _fatal);
@@ -419,7 +419,7 @@ static void via_pm_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo via_pm_info = {
-.name =
[PATCH v2 01/10] vt82c686: Rename AC97/MC97 parts from VT82C686B to VIA
These parts are common between VT82C686B and VT8231 so can be shared
in the future. Rename them to VIA prefix accordingly.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
hw/isa/vt82c686.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index b3170c70c3..2a0f85dea9 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -168,22 +168,22 @@ struct VT686PMState {
uint32_t smb_io_base;
};
-struct VT686AC97State {
+struct VIAAC97State {
PCIDevice dev;
};
-struct VT686MC97State {
+struct VIAMC97State {
PCIDevice dev;
};
#define TYPE_VT82C686B_PM_DEVICE "VT82C686B_PM"
OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM_DEVICE)
-#define TYPE_VT82C686B_MC97_DEVICE "VT82C686B_MC97"
-OBJECT_DECLARE_SIMPLE_TYPE(VT686MC97State, VT82C686B_MC97_DEVICE)
+#define TYPE_VIA_MC97_DEVICE "VIA_MC97"
+OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97_DEVICE)
-#define TYPE_VT82C686B_AC97_DEVICE "VT82C686B_AC97"
-OBJECT_DECLARE_SIMPLE_TYPE(VT686AC97State, VT82C686B_AC97_DEVICE)
+#define TYPE_VIA_AC97_DEVICE "VIA_AC97"
+OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97_DEVICE)
static void pm_update_sci(VT686PMState *s)
{
@@ -260,7 +260,7 @@ static const VMStateDescription vmstate_acpi = {
static void vt82c686b_ac97_realize(PCIDevice *dev, Error **errp)
{
-VT686AC97State *s = VT82C686B_AC97_DEVICE(dev);
+VIAAC97State *s = VIA_AC97_DEVICE(dev);
uint8_t *pci_conf = s->dev.config;
pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
@@ -274,7 +274,7 @@ void vt82c686b_ac97_init(PCIBus *bus, int devfn)
{
PCIDevice *dev;
-dev = pci_new(devfn, TYPE_VT82C686B_AC97_DEVICE);
+dev = pci_new(devfn, TYPE_VIA_AC97_DEVICE);
pci_realize_and_unref(dev, bus, _fatal);
}
@@ -293,9 +293,9 @@ static void via_ac97_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo via_ac97_info = {
-.name = TYPE_VT82C686B_AC97_DEVICE,
+.name = TYPE_VIA_AC97_DEVICE,
.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VT686AC97State),
+.instance_size = sizeof(VIAAC97State),
.class_init= via_ac97_class_init,
.interfaces = (InterfaceInfo[]) {
{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
@@ -305,7 +305,7 @@ static const TypeInfo via_ac97_info = {
static void vt82c686b_mc97_realize(PCIDevice *dev, Error **errp)
{
-VT686MC97State *s = VT82C686B_MC97_DEVICE(dev);
+VIAMC97State *s = VIA_MC97_DEVICE(dev);
uint8_t *pci_conf = s->dev.config;
pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_INVALIDATE |
@@ -318,7 +318,7 @@ void vt82c686b_mc97_init(PCIBus *bus, int devfn)
{
PCIDevice *dev;
-dev = pci_new(devfn, TYPE_VT82C686B_MC97_DEVICE);
+dev = pci_new(devfn, TYPE_VIA_MC97_DEVICE);
pci_realize_and_unref(dev, bus, _fatal);
}
@@ -337,9 +337,9 @@ static void via_mc97_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo via_mc97_info = {
-.name = TYPE_VT82C686B_MC97_DEVICE,
+.name = TYPE_VIA_MC97_DEVICE,
.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VT686MC97State),
+.instance_size = sizeof(VIAMC97State),
.class_init= via_mc97_class_init,
.interfaces = (InterfaceInfo[]) {
{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
--
2.21.3
[PATCH v2 04/10] vt82c686: Remove vt82c686b_[am]c97_init() functions
These are legacy init functions that are just equivalent to directly
calling pci_create_simple so do that instead. Also rename objects to
lower case via-ac97 and via-mc97 matching naming of other devices.
Signed-off-by: BALATON Zoltan
---
hw/isa/vt82c686.c | 27 ---
hw/mips/fuloong2e.c | 4 ++--
include/hw/isa/vt82c686.h | 4 ++--
3 files changed, 8 insertions(+), 27 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d40599c7da..8677a2d212 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -179,12 +179,6 @@ struct VIAMC97State {
#define TYPE_VT82C686B_PM "VT82C686B_PM"
OBJECT_DECLARE_SIMPLE_TYPE(VT686PMState, VT82C686B_PM)
-#define TYPE_VIA_MC97 "VIA_MC97"
-OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97)
-
-#define TYPE_VIA_AC97 "VIA_AC97"
-OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97)
-
static void pm_update_sci(VT686PMState *s)
{
int sci_level, pmsts;
@@ -254,10 +248,13 @@ static const VMStateDescription vmstate_acpi = {
};
/*
- * TODO: vt82c686b_ac97_init() and vt82c686b_mc97_init()
+ * TODO: VIA_AC97 and VIA_MC97
* just register a PCI device now, functionalities will be implemented later.
*/
+OBJECT_DECLARE_SIMPLE_TYPE(VIAMC97State, VIA_MC97)
+OBJECT_DECLARE_SIMPLE_TYPE(VIAAC97State, VIA_AC97)
+
static void vt82c686b_ac97_realize(PCIDevice *dev, Error **errp)
{
VIAAC97State *s = VIA_AC97(dev);
@@ -270,14 +267,6 @@ static void vt82c686b_ac97_realize(PCIDevice *dev, Error
**errp)
pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
}
-void vt82c686b_ac97_init(PCIBus *bus, int devfn)
-{
-PCIDevice *dev;
-
-dev = pci_new(devfn, TYPE_VIA_AC97);
-pci_realize_and_unref(dev, bus, _fatal);
-}
-
static void via_ac97_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -314,14 +303,6 @@ static void vt82c686b_mc97_realize(PCIDevice *dev, Error
**errp)
pci_set_long(pci_conf + PCI_INTERRUPT_PIN, 0x03);
}
-void vt82c686b_mc97_init(PCIBus *bus, int devfn)
-{
-PCIDevice *dev;
-
-dev = pci_new(devfn, TYPE_VIA_MC97);
-pci_realize_and_unref(dev, bus, _fatal);
-}
-
static void via_mc97_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index f0733e87b7..3b0489f781 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -264,8 +264,8 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
*i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1, NULL);
/* Audio support */
-vt82c686b_ac97_init(pci_bus, PCI_DEVFN(slot, 5));
-vt82c686b_mc97_init(pci_bus, PCI_DEVFN(slot, 6));
+pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
+pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
}
/* Network support */
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index f23f45dfb1..ff80a926dc 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -3,11 +3,11 @@
#define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
+#define TYPE_VIA_AC97 "via-ac97"
+#define TYPE_VIA_MC97 "via-mc97"
/* vt82c686.c */
ISABus *vt82c686b_isa_init(PCIBus * bus, int devfn);
-void vt82c686b_ac97_init(PCIBus *bus, int devfn);
-void vt82c686b_mc97_init(PCIBus *bus, int devfn);
I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
qemu_irq sci_irq);
--
2.21.3
[PATCH v2 03/10] vt82c686b: Rename VT82C686B to VT82C686B_ISA
This is really the ISA bridge part so name the type accordingly.
Signed-off-by: BALATON Zoltan
---
hw/isa/vt82c686.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 1be1169f83..d40599c7da 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -43,14 +43,14 @@ typedef struct SuperIOConfig {
uint8_t data;
} SuperIOConfig;
-struct VT82C686BState {
+struct VT82C686BISAState {
PCIDevice dev;
MemoryRegion superio;
SuperIOConfig superio_conf;
};
-#define TYPE_VT82C686B "VT82C686B"
-OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B)
+#define TYPE_VT82C686B_ISA "vt82c686b-isa"
+OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BISAState, VT82C686B_ISA)
static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
unsigned size)
@@ -117,7 +117,7 @@ static const MemoryRegionOps superio_ops = {
static void vt82c686b_isa_reset(DeviceState *dev)
{
-VT82C686BState *vt82c = VT82C686B(dev);
+VT82C686BISAState *vt82c = VT82C686B_ISA(dev);
uint8_t *pci_conf = vt82c->dev.config;
pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
@@ -146,7 +146,7 @@ static void vt82c686b_isa_reset(DeviceState *dev)
static void vt82c686b_write_config(PCIDevice *d, uint32_t address,
uint32_t val, int len)
{
-VT82C686BState *vt686 = VT82C686B(d);
+VT82C686BISAState *vt686 = VT82C686B_ISA(d);
DPRINTF("vt82c686b_write_config address 0x%x val 0x%x len 0x%x\n",
address, val, len);
@@ -434,7 +434,7 @@ static const VMStateDescription vmstate_via = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
-VMSTATE_PCI_DEVICE(dev, VT82C686BState),
+VMSTATE_PCI_DEVICE(dev, VT82C686BISAState),
VMSTATE_END_OF_LIST()
}
};
@@ -442,7 +442,7 @@ static const VMStateDescription vmstate_via = {
/* init the PCI-to-ISA bridge */
static void vt82c686b_realize(PCIDevice *d, Error **errp)
{
-VT82C686BState *vt82c = VT82C686B(d);
+VT82C686BISAState *vt82c = VT82C686B_ISA(d);
uint8_t *pci_conf;
ISABus *isa_bus;
uint8_t *wmask;
@@ -479,7 +479,7 @@ ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
{
PCIDevice *d;
-d = pci_create_simple_multifunction(bus, devfn, true, TYPE_VT82C686B);
+d = pci_create_simple_multifunction(bus, devfn, true, TYPE_VT82C686B_ISA);
return ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
}
@@ -505,9 +505,9 @@ static void via_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo via_info = {
-.name = TYPE_VT82C686B,
+.name = TYPE_VT82C686B_ISA,
.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VT82C686BState),
+.instance_size = sizeof(VT82C686BISAState),
.class_init= via_class_init,
.interfaces = (InterfaceInfo[]) {
{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
--
2.21.3
[PATCH v2 00/10] Misc vt82c686b clean ups
I'm sending this v2 with tags added and small edits after Philippe's review for now, maybe these are already good to go. I've taken out a few patches that may need some more work but I've run out of free time for now so will have to come back to them later. I still could not cleanly add the VT8231 model which may need some more reorganising and found a few issues with the existin 868B that may need to be fixed first so those left out patches may change anyway so will be included in a future series. Regards, BALATON Zoltan BALATON Zoltan (10): vt82c686: Rename AC97/MC97 parts from VT82C686B to VIA vt82c686: Remove unnecessary _DEVICE suffix from type macros vt82c686b: Rename VT82C686B to VT82C686B_ISA vt82c686: Remove vt82c686b_[am]c97_init() functions vt82c686: Split off via-[am]c97 into separate file in hw/audio audio/via-ac97: Simplify code and set user_creatable to false vt82c686: Remove legacy vt82c686b_isa_init() function vt82c686: Remove legacy vt82c686b_pm_init() function vt82c686: Convert debug printf to trace points vt82c686: Remove unneeded includes and defines hw/audio/meson.build | 1 + hw/audio/via-ac97.c | 93 hw/isa/trace-events | 6 ++ hw/isa/vt82c686.c | 217 +- hw/mips/fuloong2e.c | 13 ++- include/hw/isa/vt82c686.h | 12 +-- 6 files changed, 139 insertions(+), 203 deletions(-) create mode 100644 hw/audio/via-ac97.c -- 2.21.3
Re: [PATCH 01/12] vt82c686: Add APM and ACPI dependencies for VT82C686
OK, just do it as Philippe suggested. Huacai On Mon, Dec 28, 2020 at 9:42 AM BALATON Zoltan via wrote: > > Hello, > > On Mon, 28 Dec 2020, Huacai Chen wrote: > > Hi, BALATON > > > > On Sun, Dec 27, 2020 at 9:21 AM BALATON Zoltan wrote: > >> > >> Compiling vt82c686.c fails without APM and ACPI_PM functions. Add > >> dependency on these in Kconfig to fix this. > >> > >> Signed-off-by: BALATON Zoltan > >> --- > >> hw/isa/Kconfig | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig > >> index c7f07854f7..2ca2593ee6 100644 > >> --- a/hw/isa/Kconfig > >> +++ b/hw/isa/Kconfig > >> @@ -47,6 +47,8 @@ config VT82C686 > >> select ACPI_SMBUS > >> select SERIAL_ISA > >> select FDC > >> +select APM > >> +select ACPI_X86 > > I feel a bit uncomfortable with ACPI_X86 in the MIPS code, can we just > > select ACPI? And if that is not enough, can we select more options? > > This patch is not new, I've tried submitting it before but got rejeceted > for similar reason: > > https://lists.nongnu.org/archive/html/qemu-devel/2019-03/msg03428.html > > Then Philippe said he had a better alternative but it's still not fixed in > master so this patch is needed and you likely already depend on X86 > without knowing as something is pulling these in for MIPS. This can be > reproduced e,g, by adding this device to PPC as: > > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig > index d235a096c6..90b53d40c2 100644 > --- a/hw/ppc/Kconfig > +++ b/hw/ppc/Kconfig > @@ -64,6 +64,7 @@ config SAM460EX > select SMBUS_EEPROM > select USB_EHCI_SYSBUS > select USB_OHCI > +select VT82C686 > > config PREP > bool > > then compiling --target-list=ppc-softmmu > Even after: > > diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig > index c7f07854f7..75986671b9 100644 > --- a/hw/isa/Kconfig > +++ b/hw/isa/Kconfig > @@ -47,6 +47,8 @@ config VT82C686 > select ACPI_SMBUS > select SERIAL_ISA > select FDC > +select APM > +select ACPI > > config SMC37C669 > bool > > I get: > > [] Linking target qemu-system-ppc > FAILED: qemu-system-ppc > ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `vt82c686b_pm_realize': > hw/isa/vt82c686.c:378: undefined reference to `acpi_pm_tmr_init' > ld: hw/isa/vt82c686.c:379: undefined reference to `acpi_pm1_evt_init' > ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `pm_update_sci': > hw/isa/vt82c686.c:192: undefined reference to `acpi_pm1_evt_get_sts' > ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `vt82c686b_pm_realize': > hw/isa/vt82c686.c:380: undefined reference to `acpi_pm1_cnt_init' > ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `pm_update_sci': > hw/isa/vt82c686.c:200: undefined reference to `acpi_pm_tmr_update' > collect2: error: ld returned 1 exit status > > So my patch just makes existing dependencies explicit and allows this to > build but I'm OK with any other fix you propose that fixes the above case > as that's how I'll try to use this in the future. (I did look at this when > first found it and concluded that I could not make a better fix than > depending on ACPI_X86 here. I forgot the details but it was way more work > than I want to take up for this so please propose a better fix if you > can't accept this patch.) > > Maybe Philippe remembers some more. > > Regards, > BALATON Zoltan > -- Huacai Chen
Re: [PATCH 01/12] vt82c686: Add APM and ACPI dependencies for VT82C686
Hello, On Mon, 28 Dec 2020, Huacai Chen wrote: Hi, BALATON On Sun, Dec 27, 2020 at 9:21 AM BALATON Zoltan wrote: Compiling vt82c686.c fails without APM and ACPI_PM functions. Add dependency on these in Kconfig to fix this. Signed-off-by: BALATON Zoltan --- hw/isa/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index c7f07854f7..2ca2593ee6 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -47,6 +47,8 @@ config VT82C686 select ACPI_SMBUS select SERIAL_ISA select FDC +select APM +select ACPI_X86 I feel a bit uncomfortable with ACPI_X86 in the MIPS code, can we just select ACPI? And if that is not enough, can we select more options? This patch is not new, I've tried submitting it before but got rejeceted for similar reason: https://lists.nongnu.org/archive/html/qemu-devel/2019-03/msg03428.html Then Philippe said he had a better alternative but it's still not fixed in master so this patch is needed and you likely already depend on X86 without knowing as something is pulling these in for MIPS. This can be reproduced e,g, by adding this device to PPC as: diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig index d235a096c6..90b53d40c2 100644 --- a/hw/ppc/Kconfig +++ b/hw/ppc/Kconfig @@ -64,6 +64,7 @@ config SAM460EX select SMBUS_EEPROM select USB_EHCI_SYSBUS select USB_OHCI +select VT82C686 config PREP bool then compiling --target-list=ppc-softmmu Even after: diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig index c7f07854f7..75986671b9 100644 --- a/hw/isa/Kconfig +++ b/hw/isa/Kconfig @@ -47,6 +47,8 @@ config VT82C686 select ACPI_SMBUS select SERIAL_ISA select FDC +select APM +select ACPI config SMC37C669 bool I get: [] Linking target qemu-system-ppc FAILED: qemu-system-ppc ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `vt82c686b_pm_realize': hw/isa/vt82c686.c:378: undefined reference to `acpi_pm_tmr_init' ld: hw/isa/vt82c686.c:379: undefined reference to `acpi_pm1_evt_init' ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `pm_update_sci': hw/isa/vt82c686.c:192: undefined reference to `acpi_pm1_evt_get_sts' ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `vt82c686b_pm_realize': hw/isa/vt82c686.c:380: undefined reference to `acpi_pm1_cnt_init' ld: libcommon.fa.p/hw_isa_vt82c686.c.o: in function `pm_update_sci': hw/isa/vt82c686.c:200: undefined reference to `acpi_pm_tmr_update' collect2: error: ld returned 1 exit status So my patch just makes existing dependencies explicit and allows this to build but I'm OK with any other fix you propose that fixes the above case as that's how I'll try to use this in the future. (I did look at this when first found it and concluded that I could not make a better fix than depending on ACPI_X86 here. I forgot the details but it was way more work than I want to take up for this so please propose a better fix if you can't accept this patch.) Maybe Philippe remembers some more. Regards, BALATON Zoltan
RE: [PATCH 1/3] qapi/net: Add new QMP command for COLO passthrough
> -Original Message-
> From: Jason Wang
> Sent: Friday, December 25, 2020 2:20 PM
> To: Zhang, Chen ; qemu-dev de...@nongnu.org>; Eric Blake ; Dr. David Alan
> Gilbert ; Markus Armbruster
> Cc: Zhang Chen
> Subject: Re: [PATCH 1/3] qapi/net: Add new QMP command for COLO
> passthrough
>
>
> On 2020/12/24 上午9:09, Zhang Chen wrote:
> > From: Zhang Chen
> >
> > Since the real user scenario does not need to monitor all traffic.
> > Add colo-passthrough-add and colo-passthrough-del to maintain a COLO
> > network passthrough list.
> >
> > Signed-off-by: Zhang Chen
> > ---
> > net/net.c | 12
> > qapi/net.json | 46
> ++
> > 2 files changed, 58 insertions(+)
> >
> > diff --git a/net/net.c b/net/net.c
> > index e1035f21d1..eac7a92618 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1151,6 +1151,18 @@ void qmp_netdev_del(const char *id, Error
> **errp)
> > qemu_del_net_client(nc);
> > }
> >
> > +void qmp_colo_passthrough_add(const char *prot, const uint32_t port,
> > + Error **errp) {
> > +/* Setup passthrough connection */ }
> > +
> > +void qmp_colo_passthrough_del(const char *prot, const uint32_t port,
> > + Error **errp) {
> > +/* Delete passthrough connection */ }
> > +
> > static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
> > {
> > char *str;
> > diff --git a/qapi/net.json b/qapi/net.json index
> > c31748c87f..466c29714e 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -714,3 +714,49 @@
> > ##
> > { 'event': 'FAILOVER_NEGOTIATED',
> > 'data': {'device-id': 'str'} }
> > +
> > +##
> > +# @colo-passthrough-add:
> > +#
> > +# Add passthrough entry according to customer's needs in COLO-compare.
> > +#
> > +# @protocol: COLO passthrough just support TCP and UDP.
> > +#
> > +# @port: TCP or UDP port number.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 5.3
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-add",
> > +# "arguments": { "protocol": "tcp", "port": 3389 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-add',
> > + 'data': {'protocol': 'str', 'port': 'uint32'} }
>
>
> Do we plan to support 4-tuple (src ip,src port, dst ip, dst port) in the
> future? If
> yes, let's add them now.
>
> And do we plan to support wildcard here?
I think just using the port is enough for COLO compare.
Because in this case, users need passthrough some guest services are
distinguished by static ports.
And for support 4-tuple and wildcard are a good question, do you think we
should add some passthrough
Mechanism for all Qemu net filter? If yes, we should support that in the future.
Thanks
Chen
>
> Thanks
>
>
> > +
> > +##
> > +# @colo-passthrough-del:
> > +#
> > +# Delete passthrough entry according to customer's needs in COLO-
> compare.
> > +#
> > +# @protocol: COLO passthrough just support TCP and UDP.
> > +#
> > +# @port: TCP or UDP port number.
> > +#
> > +# Returns: Nothing on success
> > +#
> > +# Since: 5.3
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "colo-passthrough-del",
> > +# "arguments": { "protocol": "tcp", "port": 3389 } }
> > +# <- { "return": {} }
> > +#
> > +##
> > +{ 'command': 'colo-passthrough-del',
> > + 'data': {'protocol': 'str', 'port': 'uint32'} }
RE: [PATCH 0/3] Bypass specific network traffic in COLO
> -Original Message- > From: Jason Wang > Sent: Friday, December 25, 2020 2:23 PM > To: Zhang, Chen ; qemu-dev de...@nongnu.org>; Eric Blake ; Dr. David Alan > Gilbert ; Markus Armbruster > Cc: Zhang Chen > Subject: Re: [PATCH 0/3] Bypass specific network traffic in COLO > > > On 2020/12/24 上午9:09, Zhang Chen wrote: > > From: Zhang Chen > > > > Since the real user scenario does not need to monitor all traffic. > > > Hi Chen: > > It would be better to elaborate more on this. E.g what scenario and who will > use those new QMP/HMP commands. OK, I will add more commit log in next version. Thanks Chen > > Thanks > > > > This series give user ability to bypass kinds of network stream. > > > > Zhang Chen (3): > >qapi/net: Add new QMP command for COLO passthrough > >hmp-commands: Add new HMP command for COLO passthrough > >net/colo-compare: Add handler for passthrough connection > > > > hmp-commands.hx | 26 +++ > > include/monitor/hmp.h | 2 ++ > > monitor/hmp-cmds.c| 20 ++ > > net/colo-compare.c| 49 > +++ > > net/colo-compare.h| 2 ++ > > net/net.c | 39 ++ > > qapi/net.json | 46 > > > 7 files changed, 184 insertions(+) > >
Re: [PATCH 01/12] vt82c686: Add APM and ACPI dependencies for VT82C686
Hi, BALATON On Sun, Dec 27, 2020 at 9:21 AM BALATON Zoltan wrote: > > Compiling vt82c686.c fails without APM and ACPI_PM functions. Add > dependency on these in Kconfig to fix this. > > Signed-off-by: BALATON Zoltan > --- > hw/isa/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig > index c7f07854f7..2ca2593ee6 100644 > --- a/hw/isa/Kconfig > +++ b/hw/isa/Kconfig > @@ -47,6 +47,8 @@ config VT82C686 > select ACPI_SMBUS > select SERIAL_ISA > select FDC > +select APM > +select ACPI_X86 I feel a bit uncomfortable with ACPI_X86 in the MIPS code, can we just select ACPI? And if that is not enough, can we select more options? Huacai > > config SMC37C669 > bool > -- > 2.21.3 >
Re: [PATCH v2] acpi: Permit OEM ID and OEM table ID fields to be changed
Paolo Bonzini writes: > On 22/12/20 16:39, Marian Posteuca wrote: Qemu's ACPI table generation sets the fields OEM ID and OEM table ID to "BOCHS " and "BXPC" where "" is replaced by the ACPI table name. Some games like Red Dead Redemption 2 seem to check the ACPI OEM ID and OEM table ID for the strings "BOCHS" and "BXPC" and if they are found, the game crashes(this may be an intentional detection mechanism to prevent playing the game in a virtualized environment). >>> This isn't a technical question/comment about the patch itself, but >>> about something different. Do we really want to play this whack-a-mole >>> game? If we change ACPI table IDs, those who want to disallow running >>> their software inside qemu/kvm will find some other way to check for >>> this environment. We will change that, - just to be found again. And >>> so on.. is it productive? I don't think so. >> >> My personal opinion is that as long as it's not too difficult to mask >> that the guest is running in a virtualized environment we should try to >> do these changes. But I guess this can only be judged on per change basis. > > I don't have any particular opinion against the "arms > race"/"whack-a-mole" situation. We played the game (and sort of won, > they got tired of changing the drivers) against NVIDIA already. > > For 6.0 I'm already planning to revamp a bunch of machine properties, > for example making -acpitable file=xxx a synonym for "-machine > acpi.tables.N.file=xxx". Perhaps we could plan for that and make the > option "-machine acpi.oem_id". This looks like a great idea. Noob question here, should I change my patch in any way for this to happen? > > Paolo
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/27/20 5:40 PM, BALATON Zoltan via wrote:
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/25/20 12:23 AM, BALATON Zoltan wrote:
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
Not really. That patch did what it said (only emulating (half) native
mode instead of only emulating legacy mode) so it wasn't broken per se
but it turned out that approach wasn't good enough for all use cases so
this now takes a different turn (emulating either legacy or half-native
mode based on option property). Therefore. I don't think Fixes: applies
in this case. It fixes an issue with a guest but replaces previous patch
with different approach. (Even though it reuses most of it.)
Well, if Linux guest got broken by this commit, why not name it a "fix"?
We do call it a fix in the patch title. I just thought Fixes: tag was more
for either security fixes or cases when original commit had a bug that's
fixed up by this patch which is not exactly the case here.
Anyway I don't mind how it is called. I find important to refer to the
commit hash to help navigating between commits while reviewing history.
What about:
'''
The legacy mode for IDE support has been removed in commit 4ea98d317eb
("ide/via: Implement and use native PCI IDE mode"). When using a Linux
guest, the Fuloong2e machine requires the legacy mode.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
'''
Guenter, is that OK with you? (I can update when applying this series
via the MIPS tree).
I've submitted v2 with this commit message (slightly edited) mentioning
the original commit for tracking.
Regards,
BALATON Zoltan
[PATCH v2 2/2] via-ide: Fix fuloong2e support
From: Guenter Roeck
The IDE legacy mode emulation has been removed in commit 4ea98d317eb
("ide/via: Implement and use native PCI IDE mode") but some Linux
kernels (probably including def_config) require legacy mode on the
Fuloong2e so only emulating native mode did not turn out feasible.
Add property to via-ide model to make the mode configurable, and set
legacy mode for Fuloong2e.
Signed-off-by: Guenter Roeck
[balaton: Use bit in flags for property, add comment for missing BAR4]
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Guenter Roeck
---
v2: Reworded commit message
hw/ide/via.c| 19 +--
hw/mips/fuloong2e.c | 4 +++-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..7d54d7e829 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "sysemu/dma.h"
@@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>bus[1], "via-ide1-cmd", 4);
pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, >cmd_bar[1]);
-bmdma_setup_bar(d);
-pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
+if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
+/* Missing BAR4 will make Linux driver fall back to legacy PIO mode */
+bmdma_setup_bar(d);
+pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
+}
qdev_init_gpio_in(ds, via_ide_set_irq, 2);
for (i = 0; i < 2; i++) {
ide_bus_new(>bus[i], sizeof(d->bus[i]), ds, i, 2);
+if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
+ide_init_ioport(>bus[i], NULL, i ? 0x170 : 0x1f0,
+i ? 0x376 : 0x3f6);
+}
ide_init2(>bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(>bus[i], >bmdma[i], d);
@@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
}
}
+static Property via_ide_properties[] = {
+DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
+false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void via_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -223,6 +237,7 @@ static void via_ide_class_init(ObjectClass *klass, void
*data)
k->device_id = PCI_DEVICE_ID_VIA_IDE;
k->revision = 0x06;
k->class_id = PCI_CLASS_STORAGE_IDE;
+device_class_set_props(dc, via_ide_properties);
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 45c596f4fe..f0733e87b7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
/* Super I/O */
isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
+qdev_prop_set_bit(>qdev, "legacy_mode", true);
+pci_realize_and_unref(dev, pci_bus, _fatal);
pci_ide_create_devs(dev);
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
--
2.21.3
[PATCH v2 1/2] ide: Make room for flags in PCIIDEState and add one for legacy mode
We'll need a flag for implementing some device specific behaviour in
via-ide but we already have a currently CMD646 specific field that can
be repurposed for this and leave room for further flags if needed in
the future. This patch changes the "secondary" field to "flags" and
change CMD646 and its users accordingly and define a new flag for
forcing legacy mode that will be used by via-ide for now.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Guenter Roeck
Tested-by: Guenter Roeck
---
v2: Fixed typo in commit message
hw/ide/cmd646.c | 4 ++--
hw/sparc64/sun4u.c | 2 +-
include/hw/ide/pci.h | 7 ++-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c254631485..7a96016116 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -256,7 +256,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error
**errp)
pci_conf[PCI_CLASS_PROG] = 0x8f;
pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
-if (d->secondary) {
+if (d->flags & BIT(PCI_IDE_SECONDARY)) {
/* XXX: if not enabled, really disable the seconday IDE controller */
pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
}
@@ -314,7 +314,7 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
}
static Property cmd646_ide_properties[] = {
-DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
+DEFINE_PROP_BIT("secondary", PCIIDEState, flags, PCI_IDE_SECONDARY, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 0fa13a7330..c46baa9f48 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -674,7 +674,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
}
pci_dev = pci_new(PCI_DEVFN(3, 0), "cmd646-ide");
-qdev_prop_set_uint32(_dev->qdev, "secondary", 1);
+qdev_prop_set_bit(_dev->qdev, "secondary", true);
pci_realize_and_unref(pci_dev, pci_busA, _fatal);
pci_ide_create_devs(pci_dev);
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index d8384e1c42..75d1a32f6d 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -42,6 +42,11 @@ typedef struct BMDMAState {
#define TYPE_PCI_IDE "pci-ide"
OBJECT_DECLARE_SIMPLE_TYPE(PCIIDEState, PCI_IDE)
+enum {
+PCI_IDE_SECONDARY, /* used only for cmd646 */
+PCI_IDE_LEGACY_MODE
+};
+
struct PCIIDEState {
/*< private >*/
PCIDevice parent_obj;
@@ -49,7 +54,7 @@ struct PCIIDEState {
IDEBus bus[2];
BMDMAState bmdma[2];
-uint32_t secondary; /* used only for cmd646 */
+uint32_t flags;
MemoryRegion bmdma_bar;
MemoryRegion cmd_bar[2];
MemoryRegion data_bar[2];
--
2.21.3
[PATCH v2 0/2] Fix via-ide for fuloong2e
This implements the legacy-mode emulation option for via-ide which is needed for Linux on fuloong2e. I've tested that the Debian kernel now finds CD ROM and MorphOS on pegasos2 is not affected by this. v2 adds review tags and fixes BALATON Zoltan (1): ide: Make room for flags in PCIIDEState and add one for legacy mode Guenter Roeck (1): via-ide: Fix fuloong2e support hw/ide/cmd646.c | 4 ++-- hw/ide/via.c | 19 +-- hw/mips/fuloong2e.c | 4 +++- hw/sparc64/sun4u.c | 2 +- include/hw/ide/pci.h | 7 ++- 5 files changed, 29 insertions(+), 7 deletions(-) -- 2.21.3
Re: [PATCH 3/3] sam460ex: Clean up irq mapping
On 12/25/20 3:07 PM, BALATON Zoltan wrote:
> Avoid mapping multiple interrupts to the same irq. Instead map them to
> the 4 PCI interrupts and use an or-gate in the board to connect them
> to the interrupt controller. This does not fix any known problem but
> does not seem to cause a new problem either and may be cleaner at least.
>
> Signed-off-by: BALATON Zoltan
Tested-by: Guenter Roeck
> ---
> hw/ppc/Kconfig | 1 +
> hw/ppc/ppc440_pcix.c | 28 ++--
> hw/ppc/sam460ex.c| 16 +---
> 3 files changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 5893f80909..fabdb1a96f 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -58,6 +58,7 @@ config SAM460EX
> select PFLASH_CFI01
> select IDE_SII3112
> select M41T80
> +select OR_IRQ
> select PPC440
> select SM501
> select SMBUS_EEPROM
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index ee952314c8..504decbbc2 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -57,8 +57,8 @@ struct PPC440PCIXState {
> PCIDevice *dev;
> struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
> struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
> +qemu_irq irq[PCI_NUM_PINS];
> uint32_t sts;
> -qemu_irq irq;
> AddressSpace bm_as;
> MemoryRegion bm;
>
> @@ -415,24 +415,20 @@ static void ppc440_pcix_reset(DeviceState *dev)
> s->sts = 0;
> }
>
> -/* All pins from each slot are tied to a single board IRQ.
> - * This may need further refactoring for other boards. */
> static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
> {
> -trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, 0);
> -return 0;
> +int n = (irq_num + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> +
> +trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, n);
> +return n;
> }
>
> static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
> {
> -qemu_irq *pci_irq = opaque;
> +qemu_irq *pci_irqs = opaque;
>
> trace_ppc440_pcix_set_irq(irq_num);
> -if (irq_num < 0) {
> -error_report("%s: PCI irq %d", __func__, irq_num);
> -return;
> -}
> -qemu_set_irq(*pci_irq, level);
> +qemu_set_irq(pci_irqs[irq_num], level);
> }
>
> static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int
> devfn)
> @@ -472,15 +468,19 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
> **errp)
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> PPC440PCIXState *s;
> PCIHostState *h;
> +int i;
>
> h = PCI_HOST_BRIDGE(dev);
> s = PPC440_PCIX_HOST_BRIDGE(dev);
>
> -sysbus_init_irq(sbd, >irq);
> +for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
> +sysbus_init_irq(sbd, >irq[i]);
> +}
> memory_region_init(>busmem, OBJECT(dev), "pci bus memory",
> UINT64_MAX);
> h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
> - ppc440_pcix_map_irq, >irq, >busmem,
> - get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
> + ppc440_pcix_map_irq, s->irq, >busmem,
> + get_system_io(), PCI_DEVFN(0, 0),
> ARRAY_SIZE(s->irq),
> + TYPE_PCI_BUS);
>
> s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
> "ppc4xx-host-bridge");
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 14e6583eb0..59b19fbca1 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -33,6 +33,7 @@
> #include "sysemu/qtest.h"
> #include "sysemu/reset.h"
> #include "hw/sysbus.h"
> +#include "hw/or-irq.h"
> #include "hw/char/serial.h"
> #include "hw/i2c/ppc4xx_i2c.h"
> #include "hw/i2c/smbus_eeprom.h"
> @@ -292,7 +293,7 @@ static void sam460ex_init(MachineState *machine)
> SysBusDevice *sbdev;
> struct boot_info *boot_info;
> uint8_t *spd_data;
> -int success;
> +int i, success;
>
> cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> env = >env;
> @@ -382,13 +383,22 @@ static void sam460ex_init(MachineState *machine)
>
> /* PCI bus */
> ppc460ex_pcie_init(env);
> -/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
> -dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, uic[1][0]);
> +dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, NULL);
> pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> if (!pci_bus) {
> error_report("couldn't create PCI controller!");
> exit(1);
> }
> +/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
> +sbdev = SYS_BUS_DEVICE(dev);
> +dev = qdev_new(TYPE_OR_IRQ);
> +object_property_set_int(OBJECT(dev), "num-lines", PCI_NUM_PINS,
> +_fatal);
> +qdev_realize_and_unref(dev, NULL, _fatal);
> +for (i = 0; i < PCI_NUM_PINS; i++) {
> +sysbus_connect_irq(sbdev, i,
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On 12/27/20 9:24 AM, Philippe Mathieu-Daudé wrote:
> On 12/27/20 5:40 PM, BALATON Zoltan via wrote:
>> On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
>>> On 12/25/20 12:23 AM, BALATON Zoltan wrote:
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
>>>
>>> Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
>>
>> Not really. That patch did what it said (only emulating (half) native
>> mode instead of only emulating legacy mode) so it wasn't broken per se
>> but it turned out that approach wasn't good enough for all use cases so
>> this now takes a different turn (emulating either legacy or half-native
>> mode based on option property). Therefore. I don't think Fixes: applies
>> in this case. It fixes an issue with a guest but replaces previous patch
>> with different approach. (Even though it reuses most of it.)
>
> Well, if Linux guest got broken by this commit, why not name it a "fix"?
> Anyway I don't mind how it is called. I find important to refer to the
> commit hash to help navigating between commits while reviewing history.
>
> What about:
>
> '''
> The legacy mode for IDE support has been removed in commit 4ea98d317eb
> ("ide/via: Implement and use native PCI IDE mode"). When using a Linux
> guest, the Fuloong2e machine requires the legacy mode.
> Add property to via-ide driver to make the mode configurable, and set
> legacy mode for Fuloong2e.
> '''
>
> Guenter, is that OK with you? (I can update when applying this series
> via the MIPS tree).
>
Sure, I don't really care about the commit message as long as the problem
is fixed.
Guenter
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On 12/27/20 5:40 PM, BALATON Zoltan via wrote:
> On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
>> On 12/25/20 12:23 AM, BALATON Zoltan wrote:
>>> From: Guenter Roeck
>>>
>>> Fuloong2e needs to use legacy mode for IDE support to work with Linux.
>>> Add property to via-ide driver to make the mode configurable, and set
>>> legacy mode for Fuloong2e.
>>>
>>
>> Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
>
> Not really. That patch did what it said (only emulating (half) native
> mode instead of only emulating legacy mode) so it wasn't broken per se
> but it turned out that approach wasn't good enough for all use cases so
> this now takes a different turn (emulating either legacy or half-native
> mode based on option property). Therefore. I don't think Fixes: applies
> in this case. It fixes an issue with a guest but replaces previous patch
> with different approach. (Even though it reuses most of it.)
Well, if Linux guest got broken by this commit, why not name it a "fix"?
Anyway I don't mind how it is called. I find important to refer to the
commit hash to help navigating between commits while reviewing history.
What about:
'''
The legacy mode for IDE support has been removed in commit 4ea98d317eb
("ide/via: Implement and use native PCI IDE mode"). When using a Linux
guest, the Fuloong2e machine requires the legacy mode.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
'''
Guenter, is that OK with you? (I can update when applying this series
via the MIPS tree).
Thanks,
Phil.
>
> Regards,
> BALATON Zoltan
[PATCH 1/7] block/rbd: bump librbd requirement to luminous release
even luminous (version 12.2) is unmaintained for over 3 years now.
Bump the requirement to get rid of the ifdef'ry in the code.
Signed-off-by: Peter Lieven
---
block/rbd.c | 120
configure | 7 +--
2 files changed, 12 insertions(+), 115 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 9bd2bce716..650e27c351 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -55,24 +55,10 @@
* leading "\".
*/
-/* rbd_aio_discard added in 0.1.2 */
-#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 2)
-#define LIBRBD_SUPPORTS_DISCARD
-#else
-#undef LIBRBD_SUPPORTS_DISCARD
-#endif
-
#define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER)
#define RBD_MAX_SNAPS 100
-/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */
-#ifdef LIBRBD_SUPPORTS_IOVEC
-#define LIBRBD_USE_IOVEC 1
-#else
-#define LIBRBD_USE_IOVEC 0
-#endif
-
typedef enum {
RBD_AIO_READ,
RBD_AIO_WRITE,
@@ -84,7 +70,6 @@ typedef struct RBDAIOCB {
BlockAIOCB common;
int64_t ret;
QEMUIOVector *qiov;
-char *bounce;
RBDAIOCmd cmd;
int error;
struct BDRVRBDState *s;
@@ -94,7 +79,6 @@ typedef struct RADOSCB {
RBDAIOCB *acb;
struct BDRVRBDState *s;
int64_t size;
-char *buf;
int64_t ret;
} RADOSCB;
@@ -332,13 +316,9 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const
char *keypairs_json,
static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
{
-if (LIBRBD_USE_IOVEC) {
-RBDAIOCB *acb = rcb->acb;
-iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0,
- acb->qiov->size - offs);
-} else {
-memset(rcb->buf + offs, 0, rcb->size - offs);
-}
+RBDAIOCB *acb = rcb->acb;
+iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0,
+ acb->qiov->size - offs);
}
/* FIXME Deprecate and remove keypairs or make it available in QMP. */
@@ -493,13 +473,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
g_free(rcb);
-if (!LIBRBD_USE_IOVEC) {
-if (acb->cmd == RBD_AIO_READ) {
-qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size);
-}
-qemu_vfree(acb->bounce);
-}
-
acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret));
qemu_aio_unref(acb);
@@ -866,28 +839,6 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB
*rcb)
rbd_finish_bh, rcb);
}
-static int rbd_aio_discard_wrapper(rbd_image_t image,
- uint64_t off,
- uint64_t len,
- rbd_completion_t comp)
-{
-#ifdef LIBRBD_SUPPORTS_DISCARD
-return rbd_aio_discard(image, off, len, comp);
-#else
-return -ENOTSUP;
-#endif
-}
-
-static int rbd_aio_flush_wrapper(rbd_image_t image,
- rbd_completion_t comp)
-{
-#ifdef LIBRBD_SUPPORTS_AIO_FLUSH
-return rbd_aio_flush(image, comp);
-#else
-return -ENOTSUP;
-#endif
-}
-
static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
int64_t off,
QEMUIOVector *qiov,
@@ -910,21 +861,6 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
rcb = g_new(RADOSCB, 1);
-if (!LIBRBD_USE_IOVEC) {
-if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
-acb->bounce = NULL;
-} else {
-acb->bounce = qemu_try_blockalign(bs, qiov->size);
-if (acb->bounce == NULL) {
-goto failed;
-}
-}
-if (cmd == RBD_AIO_WRITE) {
-qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
-}
-rcb->buf = acb->bounce;
-}
-
acb->ret = 0;
acb->error = 0;
acb->s = s;
@@ -938,7 +874,7 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
}
switch (cmd) {
-case RBD_AIO_WRITE: {
+case RBD_AIO_WRITE:
/*
* RBD APIs don't allow us to write more than actual size, so in order
* to support growing images, we resize the image before write
@@ -950,25 +886,16 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
goto failed_completion;
}
}
-#ifdef LIBRBD_SUPPORTS_IOVEC
-r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c);
-#else
-r = rbd_aio_write(s->image, off, size, rcb->buf, c);
-#endif
+r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c);
break;
-}
case RBD_AIO_READ:
-#ifdef LIBRBD_SUPPORTS_IOVEC
-r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c);
-#else
-r = rbd_aio_read(s->image, off, size, rcb->buf, c);
-#endif
+r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c);
break;
case RBD_AIO_DISCARD:
-r = rbd_aio_discard_wrapper(s->image, off, size, c);
+r = rbd_aio_discard(s->image, off, size, c);
[PATCH 4/7] block/rbd: add bdrv_{attach,detach}_aio_context
Signed-off-by: Peter Lieven
---
block/rbd.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index a2da70e37f..27b232f4d8 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -91,6 +91,7 @@ typedef struct BDRVRBDState {
char *namespace;
uint64_t image_size;
uint64_t object_size;
+AioContext *aio_context;
} BDRVRBDState;
static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
@@ -749,6 +750,8 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
}
}
+s->aio_context = bdrv_get_aio_context(bs);
+
/* When extending regular files, we get zeros from the OS */
bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
@@ -839,8 +842,7 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB
*rcb)
rcb->ret = rbd_aio_get_return_value(c);
rbd_aio_release(c);
-replay_bh_schedule_oneshot_event(bdrv_get_aio_context(acb->common.bs),
- rbd_finish_bh, rcb);
+replay_bh_schedule_oneshot_event(acb->s->aio_context, rbd_finish_bh, rcb);
}
static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
@@ -1151,6 +1153,18 @@ static const char *const qemu_rbd_strong_runtime_opts[]
= {
NULL
};
+static void qemu_rbd_attach_aio_context(BlockDriverState *bs,
+ AioContext *new_context)
+{
+BDRVRBDState *s = bs->opaque;
+s->aio_context = new_context;
+}
+
+static void qemu_rbd_detach_aio_context(BlockDriverState *bs)
+{
+
+}
+
static BlockDriver bdrv_rbd = {
.format_name= "rbd",
.instance_size = sizeof(BDRVRBDState),
@@ -1180,6 +1194,9 @@ static BlockDriver bdrv_rbd = {
.bdrv_snapshot_goto = qemu_rbd_snap_rollback,
.bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache,
+.bdrv_attach_aio_context = qemu_rbd_attach_aio_context,
+.bdrv_detach_aio_context = qemu_rbd_detach_aio_context,
+
.strong_runtime_opts= qemu_rbd_strong_runtime_opts,
};
--
2.17.1
Re: [PATCH 07/12] vt82c686: Remove vt82c686b_isa_init() function
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
Also rename VT82C686B type to lower case to match other device names.
If possible do not split the commit description in 2 (one part in
subject and the other part here) as this is annoying to read.
Not so much in the commit log but maybe indeed on the list in email. I did
not want to repeat subject in the description but can if you prefer.
Thanks for reviewing these, I'll wait a few days to see if there are any
other comments then will send v2 with suggested changes.
Regards,
BALATON Zoltan
Signed-off-by: BALATON Zoltan
---
hw/isa/vt82c686.c | 9 -
hw/mips/fuloong2e.c | 4 +++-
include/hw/isa/vt82c686.h | 3 +--
3 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 758c54172b..1c1239cade 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -49,7 +49,6 @@ struct VT82C686BState {
SuperIOConfig superio_conf;
};
-#define TYPE_VT82C686B "VT82C686B"
OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B)
static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
@@ -367,14 +366,6 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
>superio);
}
-ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
-{
-PCIDevice *d;
-
-d = pci_create_simple_multifunction(bus, devfn, true, TYPE_VT82C686B);
-return ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
-}
-
static void via_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 3b0489f781..60d2020033 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -240,7 +240,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
ISABus *isa_bus;
PCIDevice *dev;
-isa_bus = vt82c686b_isa_init(pci_bus, PCI_DEVFN(slot, 0));
+dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
+ TYPE_VT82C686B);
Good cleanup.
Preferably using TYPE_VT82C686B_ISA:
Reviewed-by: Philippe Mathieu-Daudé
+isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(dev), "isa.0"));
assert(isa_bus);
*p_isa_bus = isa_bus;
/* Interrupt controller */
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index ff80a926dc..89e205a1be 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -1,13 +1,12 @@
#ifndef HW_VT82C686_H
#define HW_VT82C686_H
-
+#define TYPE_VT82C686B "vt82c686b"
#define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
#define TYPE_VIA_AC97 "via-ac97"
#define TYPE_VIA_MC97 "via-mc97"
/* vt82c686.c */
-ISABus *vt82c686b_isa_init(PCIBus * bus, int devfn);
I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
qemu_irq sci_irq);
Re: [PATCH 06/12] audio/via-ac97: Simplify code and set user_creatable to false
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote: Hi Zoltan, On 12/27/20 2:10 AM, BALATON Zoltan via wrote: Remove some unneded, empty code and set user_creatable to false (besides being not implemented yet, so does nothing anyway) it's also normally part of VIA south bridge chips so no need to confuse users showing them these devices. After contributing during more than 8 years you should know we try to avoid to do multiples changes in the same patch ;) Yes, in my understanding patches should be split if - it makes bisecting easier or - makes reviewing easier Which of the above appies in this case? I think these are changes that should neither brake anything (as this device doesn't don't do anyting yet) and adding user_creatable false is a one line change that's easy to review even with the other changes. If you insist I can split this into two but I didn't think that would be any better and the series was long enough already. Regards, BALATON Zoltan Signed-off-by: BALATON Zoltan --- hw/audio/via-ac97.c | 51 + 1 file changed, 19 insertions(+), 32 deletions(-)
Re: [PATCH 03/12] vt82c686: Remove unnecessary _DEVICE suffix from type macros
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
There's no reason to suffix everything with _DEVICE when the names are
already unique without it and shorter names are more readable.
Signed-off-by: BALATON Zoltan
---
hw/isa/vt82c686.c | 48 +++
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 2a0f85dea9..1be1169f83 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -49,8 +49,8 @@ struct VT82C686BState {
SuperIOConfig superio_conf;
};
-#define TYPE_VT82C686B_DEVICE "VT82C686B"
-OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B_DEVICE)
+#define TYPE_VT82C686B "VT82C686B"
+OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B)
Can we name this one VT82C686B_ISA?
Yes, actually this is coming up in later patches adding VT8231 but I still
need to think about that. Once I clean that up but maybe I can do the
rename already here.
Regards,
BALATON Zoltan
[PATCH 5/7] block/rbd: migrate from aio to coroutines
Signed-off-by: Peter Lieven
---
block/rbd.c | 247 ++--
1 file changed, 84 insertions(+), 163 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 27b232f4d8..2d77d0007f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -66,22 +66,6 @@ typedef enum {
RBD_AIO_FLUSH
} RBDAIOCmd;
-typedef struct RBDAIOCB {
-BlockAIOCB common;
-int64_t ret;
-QEMUIOVector *qiov;
-RBDAIOCmd cmd;
-int error;
-struct BDRVRBDState *s;
-} RBDAIOCB;
-
-typedef struct RADOSCB {
-RBDAIOCB *acb;
-struct BDRVRBDState *s;
-int64_t size;
-int64_t ret;
-} RADOSCB;
-
typedef struct BDRVRBDState {
rados_t cluster;
rados_ioctx_t io_ctx;
@@ -94,6 +78,13 @@ typedef struct BDRVRBDState {
AioContext *aio_context;
} BDRVRBDState;
+typedef struct RBDTask {
+BDRVRBDState *s;
+Coroutine *co;
+bool complete;
+int64_t ret;
+} RBDTask;
+
static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
BlockdevOptionsRbd *opts, bool cache,
const char *keypairs, const char *secretid,
@@ -316,13 +307,6 @@ static int qemu_rbd_set_keypairs(rados_t cluster, const
char *keypairs_json,
return ret;
}
-static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs)
-{
-RBDAIOCB *acb = rcb->acb;
-iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0,
- acb->qiov->size - offs);
-}
-
/* FIXME Deprecate and remove keypairs or make it available in QMP. */
static int qemu_rbd_do_create(BlockdevCreateOptions *options,
const char *keypairs, const char
*password_secret,
@@ -440,46 +424,6 @@ exit:
return ret;
}
-/*
- * This aio completion is being called from rbd_finish_bh() and runs in qemu
- * BH context.
- */
-static void qemu_rbd_complete_aio(RADOSCB *rcb)
-{
-RBDAIOCB *acb = rcb->acb;
-int64_t r;
-
-r = rcb->ret;
-
-if (acb->cmd != RBD_AIO_READ) {
-if (r < 0) {
-acb->ret = r;
-acb->error = 1;
-} else if (!acb->error) {
-acb->ret = rcb->size;
-}
-} else {
-if (r < 0) {
-qemu_rbd_memset(rcb, 0);
-acb->ret = r;
-acb->error = 1;
-} else if (r < rcb->size) {
-qemu_rbd_memset(rcb, r);
-if (!acb->error) {
-acb->ret = rcb->size;
-}
-} else if (!acb->error) {
-acb->ret = r;
-}
-}
-
-g_free(rcb);
-
-acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret));
-
-qemu_aio_unref(acb);
-}
-
static char *qemu_rbd_mon_host(BlockdevOptionsRbd *opts, Error **errp)
{
const char **vals;
@@ -817,88 +761,49 @@ static int qemu_rbd_resize(BlockDriverState *bs, uint64_t
size)
return 0;
}
-static const AIOCBInfo rbd_aiocb_info = {
-.aiocb_size = sizeof(RBDAIOCB),
-};
-
-static void rbd_finish_bh(void *opaque)
+static void qemu_rbd_finish_bh(void *opaque)
{
-RADOSCB *rcb = opaque;
-qemu_rbd_complete_aio(rcb);
+RBDTask *task = opaque;
+task->complete = 1;
+aio_co_wake(task->co);
}
-/*
- * This is the callback function for rbd_aio_read and _write
- *
- * Note: this function is being called from a non qemu thread so
- * we need to be careful about what we do here. Generally we only
- * schedule a BH, and do the rest of the io completion handling
- * from rbd_finish_bh() which runs in a qemu context.
- */
-static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb)
+static void qemu_rbd_completion_cb(rbd_completion_t c, RBDTask *task)
{
-RBDAIOCB *acb = rcb->acb;
-
-rcb->ret = rbd_aio_get_return_value(c);
+task->ret = rbd_aio_get_return_value(c);
rbd_aio_release(c);
-
-replay_bh_schedule_oneshot_event(acb->s->aio_context, rbd_finish_bh, rcb);
+aio_bh_schedule_oneshot(task->s->aio_context, qemu_rbd_finish_bh, task);
}
-static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
- int64_t off,
- QEMUIOVector *qiov,
- int64_t size,
- BlockCompletionFunc *cb,
- void *opaque,
- RBDAIOCmd cmd)
+static int coroutine_fn qemu_rbd_start_co(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov,
+ int flags,
+ RBDAIOCmd cmd)
{
-RBDAIOCB *acb;
-RADOSCB *rcb = NULL;
-rbd_completion_t c;
-int r;
-
BDRVRBDState *s = bs->opaque;
+RBDTask task = { .s = s, .co = qemu_coroutine_self() };
+rbd_completion_t c;
+int r, ret = -EIO;
-acb = qemu_aio_get(_aiocb_info, bs, cb, opaque);
-acb->cmd = cmd;
-
[PATCH 2/7] block/rbd: store object_size in BDRVRBDState
Signed-off-by: Peter Lieven
---
block/rbd.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 650e27c351..bc8cf8af9b 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -90,6 +90,7 @@ typedef struct BDRVRBDState {
char *snap;
char *namespace;
uint64_t image_size;
+uint64_t object_size;
} BDRVRBDState;
static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx,
@@ -663,6 +664,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
const QDictEntry *e;
Error *local_err = NULL;
char *keypairs, *secretid;
+rbd_image_info_t info;
int r;
keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs"));
@@ -727,13 +729,15 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
goto failed_open;
}
-r = rbd_get_size(s->image, >image_size);
+r = rbd_stat(s->image, , sizeof(info));
if (r < 0) {
-error_setg_errno(errp, -r, "error getting image size from %s",
+error_setg_errno(errp, -r, "error getting image info from %s",
s->image_name);
rbd_close(s->image);
goto failed_open;
}
+s->image_size = info.size;
+s->object_size = info.obj_size;
/* If we are using an rbd snapshot, we must be r/o, otherwise
* leave as-is */
@@ -945,15 +949,7 @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs,
static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVRBDState *s = bs->opaque;
-rbd_image_info_t info;
-int r;
-
-r = rbd_stat(s->image, , sizeof(info));
-if (r < 0) {
-return r;
-}
-
-bdi->cluster_size = info.obj_size;
+bdi->cluster_size = s->object_size;
return 0;
}
--
2.17.1
[PATCH 6/7] block/rbd: add write zeroes support
Signed-off-by: Peter Lieven
---
block/rbd.c | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/block/rbd.c b/block/rbd.c
index 2d77d0007f..27b4404adf 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -63,7 +63,8 @@ typedef enum {
RBD_AIO_READ,
RBD_AIO_WRITE,
RBD_AIO_DISCARD,
-RBD_AIO_FLUSH
+RBD_AIO_FLUSH,
+RBD_AIO_WRITE_ZEROES
} RBDAIOCmd;
typedef struct BDRVRBDState {
@@ -221,8 +222,12 @@ done:
static void qemu_rbd_refresh_limits(BlockDriverState *bs, Error **errp)
{
+BDRVRBDState *s = bs->opaque;
/* XXX Does RBD support AIO on less than 512-byte alignment? */
bs->bl.request_alignment = 512;
+#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
+bs->bl.pwrite_zeroes_alignment = s->object_size;
+#endif
}
@@ -695,6 +700,9 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
}
s->aio_context = bdrv_get_aio_context(bs);
+#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
+bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
+#endif
/* When extending regular files, we get zeros from the OS */
bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
@@ -808,6 +816,11 @@ static int coroutine_fn qemu_rbd_start_co(BlockDriverState
*bs,
case RBD_AIO_FLUSH:
r = rbd_aio_flush(s->image, c);
break;
+#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
+case RBD_AIO_WRITE_ZEROES:
+r = rbd_aio_write_zeroes(s->image, offset, bytes, c, 0, 0);
+break;
+#endif
default:
r = -EINVAL;
}
@@ -880,6 +893,19 @@ static int coroutine_fn
qemu_rbd_co_pdiscard(BlockDriverState *bs,
return qemu_rbd_start_co(bs, offset, count, NULL, 0, RBD_AIO_DISCARD);
}
+#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
+static int
+coroutine_fn qemu_rbd_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+ int count, BdrvRequestFlags flags)
+{
+if (!(flags & BDRV_REQ_MAY_UNMAP)) {
+return -ENOTSUP;
+}
+return qemu_rbd_start_co(bs, offset, count, NULL, flags,
+ RBD_AIO_WRITE_ZEROES);
+}
+#endif
+
static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi)
{
BDRVRBDState *s = bs->opaque;
@@ -1108,6 +1134,9 @@ static BlockDriver bdrv_rbd = {
.bdrv_co_pwritev= qemu_rbd_co_pwritev,
.bdrv_co_flush_to_disk = qemu_rbd_co_flush,
.bdrv_co_pdiscard = qemu_rbd_co_pdiscard,
+#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
+.bdrv_co_pwrite_zeroes = qemu_rbd_co_pwrite_zeroes,
+#endif
.bdrv_snapshot_create = qemu_rbd_snap_create,
.bdrv_snapshot_delete = qemu_rbd_snap_remove,
--
2.17.1
[PATCH 3/7] block/rbd: use stored image_size in qemu_rbd_getlength
Signed-off-by: Peter Lieven
---
block/rbd.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index bc8cf8af9b..a2da70e37f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -956,15 +956,7 @@ static int qemu_rbd_getinfo(BlockDriverState *bs,
BlockDriverInfo *bdi)
static int64_t qemu_rbd_getlength(BlockDriverState *bs)
{
BDRVRBDState *s = bs->opaque;
-rbd_image_info_t info;
-int r;
-
-r = rbd_stat(s->image, , sizeof(info));
-if (r < 0) {
-return r;
-}
-
-return info.size;
+return s->image_size;
}
static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs,
--
2.17.1
[PATCH 0/7] block/rbd: migrate to coroutines and add write zeroes support
this series migrates the qemu rbd driver from the old aio emulation
to native coroutines and adds write zeroes support which is important
for block operations.
To archive this we first bump the librbd requirement to the already
outdated luminous release of ceph to get rid of some wrappers and
ifdef'ry in the code.
Peter Lieven (7):
block/rbd: bump librbd requirement to luminous release
block/rbd: store object_size in BDRVRBDState
block/rbd: use stored image_size in qemu_rbd_getlength
block/rbd: add bdrv_{attach,detach}_aio_context
block/rbd: migrate from aio to coroutines
block/rbd: add write zeroes support
block/rbd: change request alignment to 1 byte
block/rbd.c | 421 +---
configure | 7 +-
2 files changed, 139 insertions(+), 289 deletions(-)
--
2.17.1
[PATCH 7/7] block/rbd: change request alignment to 1 byte
since we implement byte interfaces and librbd supports aio on byte granularity
we can lift
the 512 byte alignment.
Signed-off-by: Peter Lieven
---
block/rbd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 27b4404adf..8673e8f553 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -223,8 +223,6 @@ done:
static void qemu_rbd_refresh_limits(BlockDriverState *bs, Error **errp)
{
BDRVRBDState *s = bs->opaque;
-/* XXX Does RBD support AIO on less than 512-byte alignment? */
-bs->bl.request_alignment = 512;
#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES
bs->bl.pwrite_zeroes_alignment = s->object_size;
#endif
--
2.17.1
Re: [PATCH 09/12] vt82c686: Convert debug printf to trace points
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
Signed-off-by: BALATON Zoltan
---
hw/isa/trace-events | 6 ++
hw/isa/vt82c686.c | 51 +
2 files changed, 21 insertions(+), 36 deletions(-)
...
switch (superio_conf->index) {
case 0x00 ... 0xdf:
case 0xe4:
case 0xe5:
+case 0xe6 ... 0xe8: /* Should set base port of parallel and serial */
case 0xe9 ... 0xed:
case 0xf3:
case 0xf5:
@@ -74,18 +68,6 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr,
uint64_t data,
case 0xfd ... 0xff:
can_write = false;
break;
-case 0xe7:
-if ((data & 0xff) != 0xfe) {
-DPRINTF("change uart 1 base. unsupported yet\n");
-can_write = false;
-}
-break;
-case 0xe8:
-if ((data & 0xff) != 0xbe) {
-DPRINTF("change uart 2 base. unsupported yet\n");
-can_write = false;
-}
-break;
default:
break;
This hunk belong to a different patch (does not match the patch
description).
In a way does, in that it removes two DPRINTFs instead of converting them.
Maybe I should mention this in the commit message or could make it a
separate patch but don't know if that's worth it.
Regards,
BALATON Zoltan
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
On 12/25/20 12:23 AM, BALATON Zoltan wrote:
From: Guenter Roeck
Fuloong2e needs to use legacy mode for IDE support to work with Linux.
Add property to via-ide driver to make the mode configurable, and set
legacy mode for Fuloong2e.
Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
Not really. That patch did what it said (only emulating (half) native mode
instead of only emulating legacy mode) so it wasn't broken per se but it
turned out that approach wasn't good enough for all use cases so this now
takes a different turn (emulating either legacy or half-native mode based
on option property). Therefore. I don't think Fixes: applies in this case.
It fixes an issue with a guest but replaces previous patch with different
approach. (Even though it reuses most of it.)
Regards,
BALATON Zoltan
Signed-off-by: Guenter Roeck
[balaton: Use bit in flags for property, add comment for missing BAR4]
Signed-off-by: BALATON Zoltan
---
hw/ide/via.c| 19 +--
hw/mips/fuloong2e.c | 4 +++-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index be09912b33..7d54d7e829 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "sysemu/dma.h"
@@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
>bus[1], "via-ide1-cmd", 4);
pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, >cmd_bar[1]);
-bmdma_setup_bar(d);
-pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
+if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
+/* Missing BAR4 will make Linux driver fall back to legacy PIO mode */
+bmdma_setup_bar(d);
+pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
+}
qdev_init_gpio_in(ds, via_ide_set_irq, 2);
for (i = 0; i < 2; i++) {
ide_bus_new(>bus[i], sizeof(d->bus[i]), ds, i, 2);
+if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
+ide_init_ioport(>bus[i], NULL, i ? 0x170 : 0x1f0,
+i ? 0x376 : 0x3f6);
+}
ide_init2(>bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(>bus[i], >bmdma[i], d);
@@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
}
}
+static Property via_ide_properties[] = {
+DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
+false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void via_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -223,6 +237,7 @@ static void via_ide_class_init(ObjectClass *klass, void
*data)
k->device_id = PCI_DEVICE_ID_VIA_IDE;
k->revision = 0x06;
k->class_id = PCI_CLASS_STORAGE_IDE;
+device_class_set_props(dc, via_ide_properties);
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 45c596f4fe..f0733e87b7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int
slot, qemu_irq intc,
/* Super I/O */
isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
-dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
+qdev_prop_set_bit(>qdev, "legacy_mode", true);
+pci_realize_and_unref(dev, pci_bus, _fatal);
pci_ide_create_devs(dev);
pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/3] sam460ex: Drop unneeded dependencies
On 12/26/20 12:07 AM, BALATON Zoltan via wrote:
> Remove dependencies from KConfig that are not actually needed.
>
> Signed-off-by: BALATON Zoltan
> ---
> hw/ppc/Kconfig | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 8548f42b0d..5893f80909 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -55,7 +55,6 @@ config PPC4XX
>
> config SAM460EX
> bool
> -select PPC405
You reviewed commit def9119efe2 ("hw/ppc/Kconfig: Let the Sam460ex
board use the PowerPC 405 devices").
> select PFLASH_CFI01
> select IDE_SII3112
> select M41T80
> @@ -64,7 +63,6 @@ config SAM460EX
> select SMBUS_EEPROM
> select USB_EHCI_SYSBUS
> select USB_OHCI
> -select FDT_PPC
Correct, I guess I got confused by the _FDT() macro used in
sam460ex_load_device_tree(). So:
Fixes: b0048f76095 ("hw/ppc/Kconfig: Only select FDT helper for machines
using it")
>
> config PREP
> bool
>
Re: [PATCH 02/16] tcg/s390x: Change FACILITY representation
On 12/25/20 9:19 PM, Richard Henderson wrote: > We will shortly need to be able to check facilities beyond the > first 64. Instead of explicitly masking against s390_facilities, > create a HAVE_FACILITY macro that indexes an array. > > Reviewed-by: David Hildenbrand > Signed-off-by: Richard Henderson > --- > v2: Change name to HAVE_FACILITY (david) > --- > tcg/s390x/tcg-target.h | 29 --- > tcg/s390x/tcg-target.c.inc | 74 +++--- > 2 files changed, 52 insertions(+), 51 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 03/16] tcg/s390x: Merge TCG_AREG0 and TCG_REG_CALL_STACK into TCGReg
On 12/25/20 9:19 PM, Richard Henderson wrote: > They are rightly values in the same enumeration. > > Reviewed-by: David Hildenbrand > Signed-off-by: Richard Henderson > --- > tcg/s390x/tcg-target.h | 28 +++- > 1 file changed, 7 insertions(+), 21 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 01/16] tcg/s390x: Rename from tcg/s390
On 12/25/20 9:19 PM, Richard Henderson wrote:
> This emphasizes that we don't support s390, only 64-bit s390x hosts.
>
> Signed-off-by: Richard Henderson
> ---
> meson.build | 2 --
> tcg/{s390 => s390x}/tcg-target-conset.h | 0
> tcg/{s390 => s390x}/tcg-target-constr.h | 0
> tcg/{s390 => s390x}/tcg-target.h| 0
> tcg/{s390 => s390x}/tcg-target.c.inc| 0
> 5 files changed, 2 deletions(-)
> rename tcg/{s390 => s390x}/tcg-target-conset.h (100%)
> rename tcg/{s390 => s390x}/tcg-target-constr.h (100%)
> rename tcg/{s390 => s390x}/tcg-target.h (100%)
> rename tcg/{s390 => s390x}/tcg-target.c.inc (100%)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 2/2] via-ide: Fix fuloong2e support
On 12/25/20 12:23 AM, BALATON Zoltan wrote:
> From: Guenter Roeck
>
> Fuloong2e needs to use legacy mode for IDE support to work with Linux.
> Add property to via-ide driver to make the mode configurable, and set
> legacy mode for Fuloong2e.
>
Fixes: 4ea98d317eb ("ide/via: Implement and use native PCI IDE mode")?
> Signed-off-by: Guenter Roeck
> [balaton: Use bit in flags for property, add comment for missing BAR4]
> Signed-off-by: BALATON Zoltan
> ---
> hw/ide/via.c| 19 +--
> hw/mips/fuloong2e.c | 4 +++-
> 2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/via.c b/hw/ide/via.c
> index be09912b33..7d54d7e829 100644
> --- a/hw/ide/via.c
> +++ b/hw/ide/via.c
> @@ -26,6 +26,7 @@
>
> #include "qemu/osdep.h"
> #include "hw/pci/pci.h"
> +#include "hw/qdev-properties.h"
> #include "migration/vmstate.h"
> #include "qemu/module.h"
> #include "sysemu/dma.h"
> @@ -185,12 +186,19 @@ static void via_ide_realize(PCIDevice *dev, Error
> **errp)
>>bus[1], "via-ide1-cmd", 4);
> pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, >cmd_bar[1]);
>
> -bmdma_setup_bar(d);
> -pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
> +if (!(d->flags & BIT(PCI_IDE_LEGACY_MODE))) {
> +/* Missing BAR4 will make Linux driver fall back to legacy PIO mode
> */
> +bmdma_setup_bar(d);
> +pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
> +}
>
> qdev_init_gpio_in(ds, via_ide_set_irq, 2);
> for (i = 0; i < 2; i++) {
> ide_bus_new(>bus[i], sizeof(d->bus[i]), ds, i, 2);
> +if (d->flags & BIT(PCI_IDE_LEGACY_MODE)) {
> +ide_init_ioport(>bus[i], NULL, i ? 0x170 : 0x1f0,
> +i ? 0x376 : 0x3f6);
> +}
> ide_init2(>bus[i], qdev_get_gpio_in(ds, i));
>
> bmdma_init(>bus[i], >bmdma[i], d);
> @@ -210,6 +218,12 @@ static void via_ide_exitfn(PCIDevice *dev)
> }
> }
>
> +static Property via_ide_properties[] = {
> +DEFINE_PROP_BIT("legacy_mode", PCIIDEState, flags, PCI_IDE_LEGACY_MODE,
> +false),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void via_ide_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -223,6 +237,7 @@ static void via_ide_class_init(ObjectClass *klass, void
> *data)
> k->device_id = PCI_DEVICE_ID_VIA_IDE;
> k->revision = 0x06;
> k->class_id = PCI_CLASS_STORAGE_IDE;
> +device_class_set_props(dc, via_ide_properties);
> set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> }
>
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 45c596f4fe..f0733e87b7 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -253,7 +253,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus,
> int slot, qemu_irq intc,
> /* Super I/O */
> isa_create_simple(isa_bus, TYPE_VT82C686B_SUPERIO);
>
> -dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> +dev = pci_new(PCI_DEVFN(slot, 1), "via-ide");
> +qdev_prop_set_bit(>qdev, "legacy_mode", true);
> +pci_realize_and_unref(dev, pci_bus, _fatal);
> pci_ide_create_devs(dev);
>
> pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
>
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 09/12] vt82c686: Convert debug printf to trace points
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
> Signed-off-by: BALATON Zoltan
> ---
> hw/isa/trace-events | 6 ++
> hw/isa/vt82c686.c | 51 +
> 2 files changed, 21 insertions(+), 36 deletions(-)
...
> switch (superio_conf->index) {
> case 0x00 ... 0xdf:
> case 0xe4:
> case 0xe5:
> +case 0xe6 ... 0xe8: /* Should set base port of parallel and serial */
> case 0xe9 ... 0xed:
> case 0xf3:
> case 0xf5:
> @@ -74,18 +68,6 @@ static void superio_ioport_writeb(void *opaque, hwaddr
> addr, uint64_t data,
> case 0xfd ... 0xff:
> can_write = false;
> break;
> -case 0xe7:
> -if ((data & 0xff) != 0xfe) {
> -DPRINTF("change uart 1 base. unsupported yet\n");
> -can_write = false;
> -}
> -break;
> -case 0xe8:
> -if ((data & 0xff) != 0xbe) {
> -DPRINTF("change uart 2 base. unsupported yet\n");
> -can_write = false;
> -}
> -break;
> default:
> break;
This hunk belong to a different patch (does not match the patch
description).
Re: [PATCH 02/12] vt82c686: Rename AC97/MC97 parts from VT82C686B to VIA
On 12/27/20 2:10 AM, BALATON Zoltan via wrote: > These parts are common between VT82C686B and VT8231 so can be shared > in the future. Rename them to VIA prefix accordingly. > > Signed-off-by: BALATON Zoltan > --- > hw/isa/vt82c686.c | 28 ++-- > 1 file changed, 14 insertions(+), 14 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 05/12] vt82c686: Split off via-[am]c97 into separate file in hw/audio
On 12/27/20 2:10 AM, BALATON Zoltan via wrote: > These supposed to implement audio part used in VIA south bridges so > they are better placed under hw/audio. "The via-[am]c97 code is supposed to implement the audio part of VIA south bridge chipset so is better placed under hw/audio/. Split it off into a separate file." Good idea! Reviewed-by: Philippe Mathieu-Daudé > > Signed-off-by: BALATON Zoltan > --- > hw/audio/meson.build | 1 + > hw/audio/via-ac97.c | 106 +++ > hw/isa/vt82c686.c| 91 - > 3 files changed, 107 insertions(+), 91 deletions(-) > create mode 100644 hw/audio/via-ac97.c
Re: [PATCH 06/12] audio/via-ac97: Simplify code and set user_creatable to false
Hi Zoltan, On 12/27/20 2:10 AM, BALATON Zoltan via wrote: > Remove some unneded, empty code and set user_creatable to false > (besides being not implemented yet, so does nothing anyway) it's also > normally part of VIA south bridge chips so no need to confuse users > showing them these devices. After contributing during more than 8 years you should know we try to avoid to do multiples changes in the same patch ;) > > Signed-off-by: BALATON Zoltan > --- > hw/audio/via-ac97.c | 51 + > 1 file changed, 19 insertions(+), 32 deletions(-)
Re: [PATCH 10/12] vt82c686: Remove unneeded includes and defines
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
> These are not used or not needed.
>
> Signed-off-by: BALATON Zoltan
> ---
> hw/isa/vt82c686.c | 8
> 1 file changed, 8 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 789459bcae..6dff2bc67d 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -12,22 +12,16 @@
>
> #include "qemu/osdep.h"
> #include "hw/isa/vt82c686.h"
> -#include "hw/i2c/i2c.h"
Maybe squash this one in patch 8 "Remove vt82c686b_pm_init"?
> #include "hw/pci/pci.h"
> #include "hw/qdev-properties.h"
> -#include "hw/isa/isa.h"
Used for isa_bus_new().
> #include "hw/isa/superio.h"
> -#include "hw/sysbus.h"
OK.
> #include "migration/vmstate.h"
> -#include "hw/mips/mips.h"
Indeed I can't see anything in commit edf79e66c02 ("Initial support
of vt82686b south bridge used by fulong mini pc") requiring it.
> #include "hw/isa/apm.h"
> #include "hw/acpi/acpi.h"
> #include "hw/i2c/pm_smbus.h"
> #include "qapi/error.h"
> -#include "qemu/module.h"
type_init().
> #include "qemu/timer.h"
> #include "exec/address-spaces.h"
> -#include "qom/object.h"
OK.
> #include "trace.h"
>
> typedef struct SuperIOConfig {
> @@ -137,8 +131,6 @@ static void vt82c686b_write_config(PCIDevice *d, uint32_t
> addr,
> }
> }
>
> -#define ACPI_DBG_IO_ADDR 0xb044
> -
> struct VT686PMState {
> PCIDevice dev;
> MemoryRegion io;
>
Re: [PATCH 08/12] vt82c686: Remove vt82c686b_pm_init() function
On 12/27/20 2:10 AM, BALATON Zoltan via wrote: > Also rename VT82C686B_PM to match other device names. s/Also/Remove vt82c686b_pm_init() function and/ Preferably copying the subject in the description to ease reading: Reviewed-by: Philippe Mathieu-Daudé > > Signed-off-by: BALATON Zoltan > --- > hw/isa/vt82c686.c | 17 - > hw/mips/fuloong2e.c | 5 - > include/hw/isa/vt82c686.h | 5 + > 3 files changed, 5 insertions(+), 22 deletions(-)
Re: [PATCH 07/12] vt82c686: Remove vt82c686b_isa_init() function
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
> Also rename VT82C686B type to lower case to match other device names.
If possible do not split the commit description in 2 (one part in
subject and the other part here) as this is annoying to read.
>
> Signed-off-by: BALATON Zoltan
> ---
> hw/isa/vt82c686.c | 9 -
> hw/mips/fuloong2e.c | 4 +++-
> include/hw/isa/vt82c686.h | 3 +--
> 3 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 758c54172b..1c1239cade 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -49,7 +49,6 @@ struct VT82C686BState {
> SuperIOConfig superio_conf;
> };
>
> -#define TYPE_VT82C686B "VT82C686B"
> OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B)
>
> static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
> @@ -367,14 +366,6 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
> >superio);
> }
>
> -ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
> -{
> -PCIDevice *d;
> -
> -d = pci_create_simple_multifunction(bus, devfn, true, TYPE_VT82C686B);
> -return ISA_BUS(qdev_get_child_bus(DEVICE(d), "isa.0"));
> -}
> -
> static void via_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 3b0489f781..60d2020033 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -240,7 +240,9 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus,
> int slot, qemu_irq intc,
> ISABus *isa_bus;
> PCIDevice *dev;
>
> -isa_bus = vt82c686b_isa_init(pci_bus, PCI_DEVFN(slot, 0));
> +dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
> + TYPE_VT82C686B);
Good cleanup.
Preferably using TYPE_VT82C686B_ISA:
Reviewed-by: Philippe Mathieu-Daudé
> +isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(dev), "isa.0"));
> assert(isa_bus);
> *p_isa_bus = isa_bus;
> /* Interrupt controller */
> diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
> index ff80a926dc..89e205a1be 100644
> --- a/include/hw/isa/vt82c686.h
> +++ b/include/hw/isa/vt82c686.h
> @@ -1,13 +1,12 @@
> #ifndef HW_VT82C686_H
> #define HW_VT82C686_H
>
> -
> +#define TYPE_VT82C686B "vt82c686b"
> #define TYPE_VT82C686B_SUPERIO "vt82c686b-superio"
> #define TYPE_VIA_AC97 "via-ac97"
> #define TYPE_VIA_MC97 "via-mc97"
>
> /* vt82c686.c */
> -ISABus *vt82c686b_isa_init(PCIBus * bus, int devfn);
> I2CBus *vt82c686b_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
>qemu_irq sci_irq);
>
>
Re: [PATCH 03/12] vt82c686: Remove unnecessary _DEVICE suffix from type macros
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
> There's no reason to suffix everything with _DEVICE when the names are
> already unique without it and shorter names are more readable.
>
> Signed-off-by: BALATON Zoltan
> ---
> hw/isa/vt82c686.c | 48 +++
> 1 file changed, 23 insertions(+), 25 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 2a0f85dea9..1be1169f83 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -49,8 +49,8 @@ struct VT82C686BState {
> SuperIOConfig superio_conf;
> };
>
> -#define TYPE_VT82C686B_DEVICE "VT82C686B"
> -OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B_DEVICE)
> +#define TYPE_VT82C686B "VT82C686B"
> +OBJECT_DECLARE_SIMPLE_TYPE(VT82C686BState, VT82C686B)
Can we name this one VT82C686B_ISA?
Re: [PATCH 03/12] vt82c686: Remove unnecessary _DEVICE suffix from type macros
On 12/27/20 2:10 AM, BALATON Zoltan via wrote: > There's no reason to suffix everything with _DEVICE when the names are > already unique without it and shorter names are more readable. > > Signed-off-by: BALATON Zoltan > --- > hw/isa/vt82c686.c | 48 +++ > 1 file changed, 23 insertions(+), 25 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 11/12] vt82c686: Rename some functions to better show where they belong
On 12/27/20 2:10 AM, BALATON Zoltan via wrote: > This groups identifiers related to the ISA bridge part and superio > part also in their naming. > > Signed-off-by: BALATON Zoltan > --- > hw/isa/vt82c686.c | 48 ++- > hw/mips/fuloong2e.c | 2 +- > include/hw/isa/vt82c686.h | 2 +- > 3 files changed, 24 insertions(+), 28 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/2] ide: Make room for flags in PCIIDEState and add one for legacy mode
On 12/25/20 12:23 AM, BALATON Zoltan wrote: > We'll need a flag for implementing some device specific behaviour in > via-ide but we already have a currently CMD646 specific field that can > be repurposed for this and leave room for furhter flags if needed in > the future. This patch changes the "secondary" field to "flags" and > change CMD646 and its users accordingly and define a new flag for > forcing legacy mode that will be used by via-ide for now. > > Signed-off-by: BALATON Zoltan > --- > hw/ide/cmd646.c | 4 ++-- > hw/sparc64/sun4u.c | 2 +- > include/hw/ide/pci.h | 7 ++- > 3 files changed, 9 insertions(+), 4 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 0/8] Fix memory leak of some device state in migration
On Sat, Dec 26, 2020 at 06:33:39PM +0800, g00517791 wrote:
> From: Jinhao Gao
>
> For some device state having some fields of VMS_ALLOC flag, they don't
> free memory allocated for the fields in vmstate_save_state and vmstate
> _load_state. We add funcs or sentences of free memory before allocation
> of memory or after load of memory to avoid memory leak.
>
Isn't there a way to handle it centrally?
IIUC the issue is repeated loads in case a load fails, right?
So can't we do something along the lines of:
Signed-off-by: Michael S. Tsirkin
diff --git a/migration/vmstate.c b/migration/vmstate.c
index e9d2aef66b..873f76739f 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -70,6 +70,7 @@ static void vmstate_handle_alloc(void *ptr, const
VMStateField *field,
gsize size = vmstate_size(opaque, field);
size *= vmstate_n_elems(opaque, field);
if (size) {
+g_free(*(void **)ptr);
*(void **)ptr = g_malloc(size);
}
}
--
MST
[Bug 1909392] [NEW] qemu-arm crashes (SIGSEGV) when executing push instruction
Public bug reported:
Dear all,
I am afraid I found a problem, it seems like qemu-arm crashes when executing
assembly push instruction.
I use qemu version 5.2.0, but it checked an older version (4.2.1) and the
problem was also present. I start qemu using "qemu-arm -cpu cortex-m4
-singlestep -g 1234 "
Callstack before crash (host)
#0 0x5575961f in stl_he_p (ptr=0x2002fffc, v=0) at
/home/faust1002/Programming/qemu/qemu-5.2.0/include/qemu/bswap.h:353
#1 0x55759716 in stl_le_p (ptr=0x2002fffc, v=0) at
/home/faust1002/Programming/qemu/qemu-5.2.0/include/qemu/bswap.h:395
#2 0x5575d3c3 in tcg_qemu_tb_exec (env=0x55d28050,
tb_ptr=0x7fffe800010a "\r\b") at ../tcg/tci.c:1221
#3 0x556bd982 in cpu_tb_exec (cpu=0x55d1fd70, itb=0x7fffe800)
at ../accel/tcg/cpu-exec.c:178
#4 0x556be57e in cpu_loop_exec_tb (cpu=0x55d1fd70,
tb=0x7fffe800, last_tb=0x7fffd8a8, tb_exit=0x7fffd8a0) at
../accel/tcg/cpu-exec.c:658
#5 0x556be7ea in cpu_exec (cpu=0x55d1fd70) at
../accel/tcg/cpu-exec.c:771
#6 0x5560af1d in cpu_loop (env=0x55d28050) at
../linux-user/arm/cpu_loop.c:237
#7 0x557415a7 in main (argc=7, argv=0x7fffe0f8,
envp=0x7fffe138) at ../linux-user/main.c:861
Callstack before crash (target)
Program received signal SIGSEGV, Segmentation fault.
Reset_Handler () at startup.s:48
48push {r14}
Please find the elf file I use attached.
Kind regards
** Affects: qemu
Importance: Undecided
Status: New
** Attachment added: "test_binary.elf"
https://bugs.launchpad.net/bugs/1909392/+attachment/5447174/+files/test_binary.elf
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1909392
Title:
qemu-arm crashes (SIGSEGV) when executing push instruction
Status in QEMU:
New
Bug description:
Dear all,
I am afraid I found a problem, it seems like qemu-arm crashes when executing
assembly push instruction.
I use qemu version 5.2.0, but it checked an older version (4.2.1) and the
problem was also present. I start qemu using "qemu-arm -cpu cortex-m4
-singlestep -g 1234 "
Callstack before crash (host)
#0 0x5575961f in stl_he_p (ptr=0x2002fffc, v=0) at
/home/faust1002/Programming/qemu/qemu-5.2.0/include/qemu/bswap.h:353
#1 0x55759716 in stl_le_p (ptr=0x2002fffc, v=0) at
/home/faust1002/Programming/qemu/qemu-5.2.0/include/qemu/bswap.h:395
#2 0x5575d3c3 in tcg_qemu_tb_exec (env=0x55d28050,
tb_ptr=0x7fffe800010a "\r\b") at ../tcg/tci.c:1221
#3 0x556bd982 in cpu_tb_exec (cpu=0x55d1fd70,
itb=0x7fffe800) at ../accel/tcg/cpu-exec.c:178
#4 0x556be57e in cpu_loop_exec_tb (cpu=0x55d1fd70,
tb=0x7fffe800, last_tb=0x7fffd8a8, tb_exit=0x7fffd8a0) at
../accel/tcg/cpu-exec.c:658
#5 0x556be7ea in cpu_exec (cpu=0x55d1fd70) at
../accel/tcg/cpu-exec.c:771
#6 0x5560af1d in cpu_loop (env=0x55d28050) at
../linux-user/arm/cpu_loop.c:237
#7 0x557415a7 in main (argc=7, argv=0x7fffe0f8,
envp=0x7fffe138) at ../linux-user/main.c:861
Callstack before crash (target)
Program received signal SIGSEGV, Segmentation fault.
Reset_Handler () at startup.s:48
48push {r14}
Please find the elf file I use attached.
Kind regards
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1909392/+subscriptions
