Re: [PATCH v2] piix: fix regression during unplug in Xen HVM domUs
On Wed, 10 May 2023, Olaf Hering wrote: > Wed, 10 May 2023 00:58:27 +0200 Olaf Hering : > > > In my debugging (with v8.0.0) it turned out the three pci_set_word > > causes the domU to hang. In fact, it is just the last one: > > > >pci_set_byte(pci_conf + 0x20, 0x01); /* BMIBA: 20-23h */ > > > > It changes the value from 0xc121 to 0x1. > > If I disable just "pci_set_word(pci_conf + PCI_COMMAND, 0x);" it works as > well. > It changes the value from 0x5 to 0. > > In general I feel it is wrong to fiddle with PCI from the host side. > This is most likely not the intention of the Xen unplug protocol. > I'm sure the guest does not expect such changes under the hood. > It happens to work by luck with pvops kernels because their PCI discovery > is done after the unplug. > > So, what do we do here to get this off the table? I don't have a concrete suggestion because I don't understand the root cause of the issue. Looking back at Paolo's reply from 2021 https://marc.info/?l=xen-devel=161669099305992=2 I think he was right. We can either fix the root cause of the issue or avoid calling qdev_reset_all on unplug. I am OK with either one.
Re: [PATCH 2/8] block/export: Fix null pointer dereference in error path
On Fri, May 12, 2023 at 11:16:03AM -0500, Eric Blake wrote: > > > On Wed, May 10, 2023 at 10:35:55PM +0200, Kevin Wolf wrote: > > > > There are some error paths in blk_exp_add() that jump to 'fail:' before > > 'exp' is even created. So we can't just unconditionally access exp->blk. > > > > Add a NULL check, and switch from exp->blk to blk, which is available > > earlier, just to be extra sure that we really cover all cases where > > BlockDevOps could have been set for it (in practice, this only happens > > in drv->create() today, so this part of the change isn't strictly > > necessary). > > > > Fixes: de79b52604e43fdeba6cee4f5af600b62169f2d2 > > Sorry for missing that on my first review, and this does look better. > > I'm assuming you plan to take this in with the rest of the series > through your tree, but let me know if I should push it faster through > the NBD tree. Because iotest: ./check 307 -nbd fails without this patch but passes with it, Tested-by: Eric Blake [and I should really remember to run more iotests than just the subset run by 'make check' when preparing a pull request...] -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH 8/8] graph-lock: Honour read locks even in the main thread
On Wed, May 10, 2023 at 10:36:01PM +0200, Kevin Wolf wrote: > > There are some conditions under which we don't actually need to do > anything for taking a reader lock: Writing the graph is only possible > from the main context while holding the BQL. So if a reader is running > in the main context under the BQL and knows that it won't be interrupted > until the next writer runs, we don't actually need to do anything. > > This is the case if the reader code neither has a nested event loop > (this is forbidden anyway while you hold the lock) nor is a coroutine > (because a writer could run when the coroutine has yielded). > > These conditions are exactly what bdrv_graph_rdlock_main_loop() asserts. > They are not fulfilled in bdrv_graph_co_rdlock(), which always runs in a > coroutine. > > This deletes the shortcuts in bdrv_graph_co_rdlock() that skip taking > the reader lock in the main thread. > > Reported-by: Fiona Ebner > Signed-off-by: Kevin Wolf > --- > block/graph-lock.c | 10 -- > 1 file changed, 10 deletions(-) Bug fix by deletion is always fun. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH 7/8] blockjob: Adhere to rate limit even when reentered early
On Wed, May 10, 2023 at 10:36:00PM +0200, Kevin Wolf wrote:
>
> When jobs are sleeping, for example to enforce a given rate limit, they
> can be reentered early, in particular in order to get paused, to update
> the rate limit or to get cancelled.
>
> Before this patch, they behave in this case as if they had fully
> completed their rate limiting delay. This means that requests are sped
> up beyond their limit, violating the constraints that the user gave us.
Aha - our tests ARE non-deterministic! Good find.
>
> Change the block jobs to sleep in a loop until the necessary delay is
> completed, while still allowing cancelling them immediately as well
> pausing (handled by the pause point in job_sleep_ns()) and updating the
> rate limit.
>
> This change is also motivated by iotests cases being prone to fail
> because drain operations pause and unpause them so often that block jobs
> complete earlier than they are supposed to. In particular, the next
> commit would fail iotests 030 without this change.
>
> Signed-off-by: Kevin Wolf
> ---
> include/block/blockjob_int.h | 14 ++
> block/commit.c | 7 ++-
> block/mirror.c | 23 ++-
> block/stream.c | 7 ++-
> blockjob.c | 22 --
> 5 files changed, 44 insertions(+), 29 deletions(-)
>
> +++ b/blockjob.c
> @@ -319,10 +319,28 @@ static bool block_job_set_speed(BlockJob *job, int64_t
> speed, Error **errp)
> return block_job_set_speed_locked(job, speed, errp);
> }
>
> -int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n)
> +void block_job_ratelimit_processed_bytes(BlockJob *job, uint64_t n)
> {
> IO_CODE();
> -return ratelimit_calculate_delay(>limit, n);
> +ratelimit_calculate_delay(>limit, n);
> +}
> +
> +void block_job_ratelimit_sleep(BlockJob *job)
> +{
> +uint64_t delay_ns;
> +
> +/*
> + * Sleep at least once. If the job is reentered early, keep waiting until
> + * we've waited for the full time that is necessary to keep the job at
> the
> + * right speed.
> + *
> + * Make sure to recalculate the delay after each (possibly interrupted)
> + * sleep because the speed can change while the job has yielded.
> + */
> +do {
> +delay_ns = ratelimit_calculate_delay(>limit, 0);
> +job_sleep_ns(>job, delay_ns);
> +} while (delay_ns && !job_is_cancelled(>job));
> }
Yes, that looks more robust.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH 6/8] test-bdrv-drain: Call bdrv_co_unref() in coroutine context
On Wed, May 10, 2023 at 10:35:59PM +0200, Kevin Wolf wrote:
>
> bdrv_unref() is a no_coroutine_fn, so calling it from coroutine context
> is invalid. Use bdrv_co_unref() instead.
>
> Signed-off-by: Kevin Wolf
> ---
> tests/unit/test-bdrv-drain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake
>
> diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
> index ae4299ccfa..08bb0f9984 100644
> --- a/tests/unit/test-bdrv-drain.c
> +++ b/tests/unit/test-bdrv-drain.c
> @@ -1019,7 +1019,7 @@ static void coroutine_fn test_co_delete_by_drain(void
> *opaque)
> g_assert_cmpint(bs->refcnt, ==, 1);
>
> if (!dbdd->detach_instead_of_delete) {
> -blk_unref(blk);
> +blk_co_unref(blk);
> } else {
> BdrvChild *c, *next_c;
> QLIST_FOREACH_SAFE(c, >children, next, next_c) {
> --
> 2.40.1
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH 5/8] test-bdrv-drain: Take graph lock more selectively
On Wed, May 10, 2023 at 10:35:58PM +0200, Kevin Wolf wrote: > > If we take a reader lock, we can't call any functions that take a writer > lock internally without causing deadlocks once the reader lock is > actually enforced in the main thread, too. Take the reader lock only > where it is actually needed. > > Signed-off-by: Kevin Wolf > --- > tests/unit/test-bdrv-drain.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 07/19] cutils: Adjust signature of parse_uint[_full]
On Thu, May 11, 2023 at 09:10:21PM -0500, Eric Blake wrote: > > It's already confusing that we have two very similar functions for > wrapping the parse of a 64-bit unsigned value, differing mainly on > whether they permit leading '-'. Adjust the signature of parse_uint() > and parse_uint_full() to be like all of qemu_strto*(): put the result > parameter last, use the same types (uint64_t is not always the same as > unsigned long long, and mark endptr const (only latter affects the I blame my late night editing. Looks better as: ...use the same types (uint64_t and unsigned long long have the same width, but are not always the same type), and mark endptr const (this latter change only affects the... > rare caller of parse_uint). Adjust all callers in the tree. > > Signed-off-by: Eric Blake > --- > include/qemu/cutils.h | 5 +- > audio/audio_legacy.c | 4 +- > block/gluster.c | 4 +- > block/nfs.c | 4 +- > blockdev.c| 4 +- > contrib/ivshmem-server/main.c | 4 +- > qapi/opts-visitor.c | 10 +-- > tests/unit/test-cutils.c | 113 +++--- > ui/vnc.c | 4 +- > util/cutils.c | 13 ++-- > util/guest-random.c | 4 +- > util/qemu-sockets.c | 10 +-- > 12 files changed, 82 insertions(+), 97 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH 4/8] qemu-img: Take graph lock more selectively
On Wed, May 10, 2023 at 10:35:57PM +0200, Kevin Wolf wrote: > > If we take a reader lock, we can't call any functions that take a writer > lock internally without causing deadlocks once the reader lock is > actually enforced in the main thread, too. Take the reader lock only > where it is actually needed. > > Signed-off-by: Kevin Wolf > --- > qemu-img.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH 3/8] qcow2: Unlock the graph in qcow2_do_open() where necessary
On Wed, May 10, 2023 at 10:35:56PM +0200, Kevin Wolf wrote: > > qcow2_do_open() calls a few no_co_wrappers that wrap functions taking > the graph lock internally as a writer. Therefore, it can't hold the > reader lock across these calls, it causes deadlocks. Drop the lock > temporarily around the calls. > > Signed-off-by: Kevin Wolf > --- > block/qcow2.c | 6 ++ > 1 file changed, 6 insertions(+) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/8] block/export: Fix null pointer dereference in error path
On Wed, May 10, 2023 at 10:35:55PM +0200, Kevin Wolf wrote:
>
> There are some error paths in blk_exp_add() that jump to 'fail:' before
> 'exp' is even created. So we can't just unconditionally access exp->blk.
>
> Add a NULL check, and switch from exp->blk to blk, which is available
> earlier, just to be extra sure that we really cover all cases where
> BlockDevOps could have been set for it (in practice, this only happens
> in drv->create() today, so this part of the change isn't strictly
> necessary).
>
> Fixes: de79b52604e43fdeba6cee4f5af600b62169f2d2
Sorry for missing that on my first review, and this does look better.
I'm assuming you plan to take this in with the rest of the series
through your tree, but let me know if I should push it faster through
the NBD tree.
> Signed-off-by: Kevin Wolf
> ---
> block/export/export.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Eric Blake
>
> diff --git a/block/export/export.c b/block/export/export.c
> index 62c7c22d45..a5c8f42f53 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -192,8 +192,10 @@ BlockExport *blk_exp_add(BlockExportOptions *export,
> Error **errp)
> return exp;
>
> fail:
> -blk_set_dev_ops(exp->blk, NULL, NULL);
> -blk_unref(blk);
> +if (blk) {
> +blk_set_dev_ops(blk, NULL, NULL);
> +blk_unref(blk);
> +}
> aio_context_release(ctx);
> if (exp) {
> g_free(exp->id);
> --
> 2.40.1
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/8] block: Call .bdrv_co_create(_opts) unlocked
On Wed, May 10, 2023 at 10:35:54PM +0200, Kevin Wolf wrote:
>
> These are functions that modify the graph, so they must be able to take
> a writer lock. This is impossible if they already hold the reader lock.
> If they need a reader lock for some of their operations, they should
> take it internally.
>
> Many of them go through blk_*(), which will always take the lock itself.
> Direct calls of bdrv_*() need to take the reader lock. Note that while
> locking for bdrv_co_*() calls is checked by TSA, this is not the case
> for the mixed_coroutine_fns bdrv_*(). Holding the lock is still required
> when they are called from coroutine context like here!
>
> This effectively reverts 4ec8df0183, but adds some internal locking
> instead.
>
> Signed-off-by: Kevin Wolf
> ---
> +++ b/block/qcow2.c
> -static int coroutine_fn
> +static int coroutine_fn GRAPH_UNLOCKED
> qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> {
> BlockdevCreateOptionsQcow2 *qcow2_opts;
> @@ -3724,8 +3726,10 @@ qcow2_co_create(BlockdevCreateOptions *create_options,
> Error **errp)
> goto out;
> }
>
> +bdrv_graph_co_rdlock();
> ret = qcow2_alloc_clusters(blk_bs(blk), 3 * cluster_size);
> if (ret < 0) {
> +bdrv_graph_co_rdunlock();
> error_setg_errno(errp, -ret, "Could not allocate clusters for qcow2 "
> "header and refcount table");
> goto out;
> @@ -3743,6 +3747,8 @@ qcow2_co_create(BlockdevCreateOptions *create_options,
> Error **errp)
>
> /* Create a full header (including things like feature table) */
> ret = qcow2_update_header(blk_bs(blk));
> +bdrv_graph_co_rdunlock();
> +
If we ever inject any 'goto out' in the elided lines, we're in
trouble. Would this be any safer by wrapping the intervening
statements in a scope-guarded lock?
But what you have is correct, despite my worries about potential
maintenance concerns.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [PULL 1/6] linux-user/sparc: Don't use 16-bit UIDs on SPARC V9
Le 12/05/2023 à 14:08, John Paul Adrian Glaubitz a écrit : Hello Laurent! On Fri, 2023-05-12 at 13:13 +0200, Laurent Vivier wrote: This patch breaks something with LTP (20230127) test fchown05_16 on sid/sparc64: tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 702 701 fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 704 fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 705 expected result; tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed Where do these tests reside? I'm not sure I know what LTP is. In any case, the patch should be correct as QEMU needs to differentiate between 32-bit and 64-bit SPARC. I agree, it could be a side effect. I didn't check. https://github.com/linux-test-project/ltp/releases/tag/20230127 Thanks, Laurent
Re: [PATCH 2/8] block/export: Fix null pointer dereference in error path
On Wed, 10 May 2023 at 21:38, Kevin Wolf wrote: > > There are some error paths in blk_exp_add() that jump to 'fail:' before > 'exp' is even created. So we can't just unconditionally access exp->blk. > > Add a NULL check, and switch from exp->blk to blk, which is available > earlier, just to be extra sure that we really cover all cases where > BlockDevOps could have been set for it (in practice, this only happens > in drv->create() today, so this part of the change isn't strictly > necessary). > > Fixes: de79b52604e43fdeba6cee4f5af600b62169f2d2 > Signed-off-by: Kevin Wolf > --- Coverity noticed this bug, incidentally: CID 1509238. -- PMM
Re: [PULL 1/6] linux-user/sparc: Don't use 16-bit UIDs on SPARC V9
Hello Laurent! On Fri, 2023-05-12 at 13:13 +0200, Laurent Vivier wrote: > This patch breaks something with LTP (20230127) test fchown05_16 on > sid/sparc64: > > tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s > fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed > fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed > fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 702 701 > fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed > fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed > fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 704 > fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed > fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed > fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 705 > > expected result; > > tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s > fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed > fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed > fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed > fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed > fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed > fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed Where do these tests reside? I'm not sure I know what LTP is. In any case, the patch should be correct as QEMU needs to differentiate between 32-bit and 64-bit SPARC. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PULL 1/6] linux-user/sparc: Don't use 16-bit UIDs on SPARC V9
On 3/30/23 15:18, Philippe Mathieu-Daudé wrote:
The 64-bit SPARC V9 syscall ABI uses 32-bit UIDs. Only enable
the 16-bit UID wrappers for 32-bit SPARC (V7 and V8).
Possibly missed in commit 992f48a036 ("Support for 32 bit
ABI on 64 bit targets (only enabled Sparc64)").
Reported-by: Gregor Riepl
Tested-by: John Paul Adrian Glaubitz
Tested-by: Zach van Rijn
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1394
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Acked-by: Laurent Vivier
Message-Id: <20230327131910.78564-1-phi...@linaro.org>
---
linux-user/syscall_defs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 614a1cbc8e..cc37054cb5 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -61,7 +61,7 @@
#if (defined(TARGET_I386) && defined(TARGET_ABI32)) \
|| (defined(TARGET_ARM) && defined(TARGET_ABI32)) \
-|| defined(TARGET_SPARC) \
+|| (defined(TARGET_SPARC) && defined(TARGET_ABI32)) \
|| defined(TARGET_M68K) || defined(TARGET_SH4) || defined(TARGET_CRIS)
/* 16 bit uid wrappers emulation */
#define USE_UID16
This patch breaks something with LTP (20230127) test fchown05_16 on sid/sparc64:
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s
fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed
fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed
fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 702 701
fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed
fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed
fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 704
fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed
fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed
fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 705
expected result;
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s
fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed
fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed
fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed
fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed
fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed
fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed
Thanks,
Laurent
Re: [PULL 04/16] aspeed/smc: Cache AspeedSMCClass
Hello,
On 5/12/23 06:00, Philippe Mathieu-Daudé wrote:
Hi Cédric,
On 25/10/22 17:20, Cédric Le Goater wrote:
Store a reference on the AspeedSMC class under the flash object and
use it when accessing the flash contents. Avoiding the class cast
checkers in these hot paths improves performance by 10% when running
the aspeed avocado tests.
I doubt you still have your benchmark number, but do you remember
if you were using --enable-qom-cast-debug ?
It is relatively easy to run.
Grab :
https://github.com/legoater/qemu-aspeed-boot/raw/master/images/ast2500-evb/buildroot-2023.02/flash.img
and run :
qemu-system-arm -M ast2500-evb,execute-in-place=true -net user -drive
file=./flash.img,format=raw,if=mtd -nographic
I tried with and without --enable-qom-cast-debug. It doesn't make
much difference.
C.
Message-Id: <20220923084803.498337-7-...@kaod.org>
Signed-off-by: Cédric Le Goater
---
include/hw/ssi/aspeed_smc.h | 2 ++
hw/ssi/aspeed_smc.c | 9 -
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 2d5f8f3d8f68..8e1dda556b91 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -30,6 +30,7 @@
#include "qom/object.h"
struct AspeedSMCState;
+struct AspeedSMCClass;
#define TYPE_ASPEED_SMC_FLASH "aspeed.smc.flash"
OBJECT_DECLARE_SIMPLE_TYPE(AspeedSMCFlash, ASPEED_SMC_FLASH)
@@ -37,6 +38,7 @@ struct AspeedSMCFlash {
SysBusDevice parent_obj;
struct AspeedSMCState *controller;
+ struct AspeedSMCClass *asc;
uint8_t cs;
MemoryRegion mmio;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index faed7e0cbe17..22df4be528a7 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -388,7 +388,7 @@ static inline int aspeed_smc_flash_cmd(const AspeedSMCFlash
*fl)
static inline int aspeed_smc_flash_addr_width(const AspeedSMCFlash *fl)
{
const AspeedSMCState *s = fl->controller;
- AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
+ AspeedSMCClass *asc = fl->asc;
if (asc->addr_width) {
return asc->addr_width(s);
@@ -420,7 +420,7 @@ static uint32_t aspeed_smc_check_segment_addr(const
AspeedSMCFlash *fl,
uint32_t addr)
{
const AspeedSMCState *s = fl->controller;
- AspeedSMCClass *asc = ASPEED_SMC_GET_CLASS(s);
+ AspeedSMCClass *asc = fl->asc;
AspeedSegments seg;
asc->reg_to_segment(s, s->regs[R_SEG_ADDR0 + fl->cs], );
@@ -1234,7 +1234,6 @@ static const TypeInfo aspeed_smc_info = {
static void aspeed_smc_flash_realize(DeviceState *dev, Error **errp)
{
AspeedSMCFlash *s = ASPEED_SMC_FLASH(dev);
- AspeedSMCClass *asc;
g_autofree char *name = g_strdup_printf(TYPE_ASPEED_SMC_FLASH ".%d",
s->cs);
if (!s->controller) {
@@ -1242,14 +1241,14 @@ static void aspeed_smc_flash_realize(DeviceState *dev,
Error **errp)
return;
}
- asc = ASPEED_SMC_GET_CLASS(s->controller);
+ s->asc = ASPEED_SMC_GET_CLASS(s->controller);
/*
* Use the default segment value to size the memory region. This
* can be changed by FW at runtime.
*/
memory_region_init_io(>mmio, OBJECT(s), _smc_flash_ops,
- s, name, asc->segments[s->cs].size);
+ s, name, s->asc->segments[s->cs].size);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mmio);
}
