RE: [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
> -Original Message- > From: Thomas Huth [mailto:th...@redhat.com] > Sent: Friday, December 11, 2020 11:24 PM > To: Peter Maydell ; qemu-devel@nongnu.org > Cc: Chenqun (kuhn) ; Richard Henderson > ; Paolo Bonzini > Subject: [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in > the softfloat tests > > The softfloat tests are external repositories, so we do not care about > implicit > fallthrough warnings in this code. > > Signed-off-by: Thomas Huth > --- Reviewed-by: Chen Qun The warnings are frequently generated for files in this directory. This is a good solution for the warnings. Thanks, Chen Qun > tests/fp/meson.build | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tests/fp/meson.build b/tests/fp/meson.build index > 3d4fb00f9d..8d739c4d59 100644 > --- a/tests/fp/meson.build > +++ b/tests/fp/meson.build > @@ -27,6 +27,7 @@ tfdir = 'berkeley-testfloat-3/source' > sfinc = include_directories(sfdir / 'include', sfspedir) > > tfcflags = [ > + '-Wno-implicit-fallthrough', >'-Wno-strict-prototypes', >'-Wno-unknown-pragmas', >'-Wno-uninitialized', > @@ -209,6 +210,7 @@ libtestfloat = static_library( > ) > > sfcflags = [ > + '-Wno-implicit-fallthrough', >'-Wno-missing-prototypes', >'-Wno-redundant-decls', >'-Wno-return-type', > -- > 2.27.0
Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
On 11/12/2020 21.10, Willian Rampazzo wrote: > On 12/11/20 2:31 PM, Thomas Huth wrote: >> Add two more simple tests to check that virtio-rng and virtio-balloon >> are at least (very) basically working on s390x. >> >> Based-on: 20201204121450.120730-1-coh...@redhat.com >> >> Thomas Huth (3): >> tests/acceptance: Extract the code to clear dmesg and wait for CRW >> reports >> tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via >> /dev/hwrng >> tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon >> device >> >> tests/acceptance/machine_s390_ccw_virtio.py | 59 +++-- >> 1 file changed, 43 insertions(+), 16 deletions(-) >> > > One observation, test_s390x_devices tends to get longer and difficult to > debug in case of problems. If a test covers one specific device type, It > will improve readability, flexibility, and debugging. In case you don't want > to spend time breaking this into multiple tests, I'll be glad to do that > after the whole series is merged. Theoretically yes, but practically we also want to run the tests as fast as possible. Quite a bit of time is used to boot the kernel, so if we add a new test for each and every device, that would increase the test time quite a bit. Thus I'd rather prefer to keep everything in one single test instead for now. > As far as code concerned, > > Reviewed-by: Willian Rampazzo > Tested-by: Willian Rampazzo Thanks! Thomas
Re: [PATCH 2/3] tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via /dev/hwrng
On 11/12/2020 21.30, Wainer dos Santos Moschetta wrote:
> Hi,
>
> On 12/11/20 2:31 PM, Thomas Huth wrote:
>> /dev/hwrng is only functional if virtio-rng is working right, so let's
>> add a sanity check for this device node.
>
> Good idea.
>
>>
>> Signed-off-by: Thomas Huth
>> ---
>> tests/acceptance/machine_s390_ccw_virtio.py | 17 +++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
>> b/tests/acceptance/machine_s390_ccw_virtio.py
>> index 733a7ca24a..7d0a78139b 100644
>> --- a/tests/acceptance/machine_s390_ccw_virtio.py
>> +++ b/tests/acceptance/machine_s390_ccw_virtio.py
>> @@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
>> '-append', kernel_command_line,
>> '-device', 'virtio-net-ccw,devno=fe.1.',
>> '-device',
>> - 'virtio-rng-ccw,devno=fe.2.,max_revision=0',
>> +
>> 'virtio-rng-ccw,devno=fe.2.,max_revision=0,id=rn1',
>> '-device',
>> - 'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
>> +
>> 'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
>> '-device', 'zpci,uid=5,target=zzz',
>> '-device', 'virtio-net-pci,id=zzz',
>> '-device', 'zpci,uid=0xa,fid=12,target=serial',
>> @@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
>> exec_command_and_wait_for_pattern(self,
>> 'cat
>> /sys/bus/ccw/devices/0.3.1234/virtio?/features',
>> virtio_rng_features)
>> + # check that /dev/hwrng works - and that it's gone after ejecting
>> + exec_command_and_wait_for_pattern(self,
>> + 'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
>> + '10+0 records out')
>> + self.clear_guests_dmesg()
>> + self.vm.command('device_del', id='rn1')
>> + self.wait_for_crw_reports()
>> + self.clear_guests_dmesg()
>> + self.vm.command('device_del', id='rn2')
>> + self.wait_for_crw_reports()
>> + exec_command_and_wait_for_pattern(self,
>> + 'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
>> + 'dd: /dev/hwrng: No such device')
>
> Maybe the expected pattern is too fragile. On my Fedora 33 system, 'dd' will
> print a different message.
We are running this test with a well-defined kernel + initrd, so I don't
think we have to care of other versions of dd here.
> What if it checks for the presence of the device file, e.g:
>
> ... self, 'test -c /dev/hwrng; echo $?', '1')
That doesn't work, the /dev/hwrng is still there (so test -c succeeds),
since this initrd uses static device nodes for this in /dev. /dev/hwrng just
can not be opened anymore after the device has been removed.
Thomas
Re: ceph + freeipa ubuntu/fedora common small bug
It's the latest ubuntu and fedora distros. On December 11, 2020 5:05:22 AM CST, "Dr. David Alan Gilbert" wrote: >* Harry G. Coin (hgc...@gmail.com) wrote: >> FYI. Same thing we saw on Fedora installing freeipa, this on ubuntu >> with ceph. Identical bitmask report. >> >> ... >> >> Fixing /var/run/ceph ownershipdone >> >> Cannot set file attribute for '/var/log/journal', value=0x0080, >> mask=0x0080, ignoring: Function not implemented >> >> Cannot set file attribute for >> '/var/log/journal/fd007229322043ad8778c214d19ed3ac', >value=0x0080, >> mask=0x0080, ignoring: Function not implemented > >This looks like it comes out of systemd's src/tmpfiles/tmpfiles.c: > >r = chattr_fd(procfs_fd, f, item->attribute_mask, NULL); >if (r < 0) >log_full_errno(IN_SET(r, -ENOTTY, -EOPNOTSUPP) ? LOG_DEBUG : >LOG_WARNING, > r, >"Cannot set file attribute for '%s', value=0x%08x, mask=0x%08x, >ignoring: %m", >path, item->attribute_value, item->attribute_mask); > >and it's chattr_fd is in it's src/basic/chattr-util.c >which is using FS_IOC_GET/SETFLAGS, which seems to be an older >way of doing things. > >Now, is that supposed to promote itself to a newer call or is it OK? > >Dave > >> ... >> >> Host has xattrs on, btrfs. >> >> >> >-- >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [PATCH] icount: improve exec nocache usage
On 12.12.2020 00:41, Richard Henderson wrote:
On 12/8/20 3:10 AM, Pavel Dovgalyuk wrote:
cpu-exec tries to execute TB without caching when current
icount budget is over. But sometimes refilled budget is big
enough to try executing cached blocks.
This patch checks that instruction budget is big enough
for next block execution instead of just running cpu_exec_nocache.
It halves the number of calls of cpu_exec_nocache function
during tested OS boot scenario.
Signed-off-by: Pavel Dovgalyuk
---
accel/tcg/cpu-exec.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 58aea605d8..251b340fb9 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -685,7 +685,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
TranslationBlock *tb,
insns_left = MIN(0x, cpu->icount_budget);
cpu_neg(cpu)->icount_decr.u16.low = insns_left;
cpu->icount_extra = cpu->icount_budget - insns_left;
-if (!cpu->icount_extra) {
+if (!cpu->icount_extra && insns_left < tb->icount) {
Reviewed-by: Richard Henderson
Thanks.
I also wonder if we should really be not caching these. Ever since MTTCG, we
have not actually been reusing the memory. We're simply removing the TB from
the hash table. I think we should be remembering these just in case we can in
fact reuse them.
I'm still thinking about reusing these blocks. Sometimes there are
loops, where blocks of small sizes like 1..3 are translated for many times.
However, we can't cache them directly, because hash table can include
only one block with the specific pc.
Pavel Dovgalyuk
Re: [PATCH] ide:atapi: check io_buffer_index in ide_atapi_cmd_reply_end
Re: [PATCH 0/7] Additional NPCM7xx devices and IPMI BMC emulation support
On Fri, Dec 11, 2020 at 04:26:03PM -0800, Havard Skinnemoen wrote: > On Fri, Dec 11, 2020 at 4:16 PM Corey Minyard wrote: > > > On Fri, Dec 11, 2020 at 12:56:07PM -0800, Hao Wu wrote: > > > Tl,dr: We'll remove the IPMI changes from the current patch set and > > > refactor > > > them in a separate patch set. > > > > > > Thank you for your review! On high level, we are trying to emulate the > > BMC > > > side of the IPMI protocol. So we cannot directly use the existing IPMI > > code. > > > However, they do have a lot in duplication as you pointed out. So we'll > > > refactor > > > the existing IPMI code and update in a way that we only add the required > > > functionality. > > > > Ah, I didn't figure that out from what you posted. So the idea is you > > can create the BMC side of the system in one qemu session with your > > changes and then you connect it to a host system running qemu with the > > host side of the interface. > > > > The wire protocol is basically symmetric, but the command handling will > > need to be separate. So you probably want to split out the base > > protocol from ipmi_bmc_extern into its own file and use that from your > > own file, to avoid the duplication. > > > > You need to do proper ATTN handling on the BMC side. And you will also > > need ties into GPIOs and whatnot for doing the reset, NMI, etc. > > > > "ipmi_host" is probably not the best name. At least to me that implied > > the host side of the interface. I'm not coming up with something I > > really like, though. Maybe "bmc_host"? That's more descriptive, though > > I'm sure a better name exists. Then you could have "bmc_host_extern" > > for the protocol. If you come up with a better naming scheme, the > > existing files can be renamed, too. > > > > The naming is my fault. > > My thinking was that ipmi-host-extern is to the BMC what ipmi-bmc-extern is > to the host. That is, the former represents the host as seen by the BMC, > and the latter represents the BMC as seen by the host. Yeah, I kind of figured that, which is why I suggested that the existing files can be renamed. I really like it when I can look at a directory and tell what everything does. Unfortunately, that's generally harder to achieve than it sounds. We could also create a separate bmc directory under hw. Then the names you have make more sense. It would also make things a bit cleaner, since I wouldn't really be maintaining the BMC side of things. I'm already dealing with that on the Linux side. > > I sent some docs to the list earlier this year, but sadly, I never got > around to follow up. You can see the generated docs here: > > https://hskinnemoen.github.io/qemu/specs/ipmi.html > > Hao, perhaps you should include my documentation patches in your next IPMI > series? If we come up with a better naming scheme for both sides, I can > update the docs accordingly. I remember that now. Yes, documentation would be good. Thanks, -corey > > Havard > > > > Thanks, > > > > -corey > > > > > > > > As for the KCS module, the BMC side of the protocol is the opposite > > > direction > > > of the existing ipmi_kcs.c code which is on the host/CPU side. For > > example, > > > in READ_STATE the CPU would read data while the BMC would write data. > > > So we can't directly use the same implementation. (They're different > > files > > > in > > > Linux either.) However, we can refactor it to re-use some of the common > > > definitions. > > > > > > We would like to remove the IPMI and KCS stuff from the current patch > > set. > > > We'll send the refactored code in a separate patch set after addressing > > > your concerns. > > > > > > Thanks again for the review! > > > > > > On Thu, Dec 10, 2020 at 7:04 PM Corey Minyard wrote: > > > > > > > On Thu, Dec 10, 2020 at 05:51:49PM -0800, Hao Wu wrote: > > > > > This patch series include a few more NPCM7XX devices including > > > > > > > > > > - Analog Digital Converter (ADC) > > > > > - Pulse Width Modulation (PWM) > > > > > - Keyboard Style Controller (KSC) > > > > > > > > > > To utilize these modules we also add two extra functionalities: > > > > > > > > > > 1. We modified the CLK module to generate clock values using > > qdev_clock. > > > > >These clocks are used to determine various clocks in NPCM7XX > > devices. > > > > > 2. We added support for emulating IPMI responder devices in BMC > > machines, > > > > >similar to the existing IPMI device support for CPU emulation. > > This > > > > allows > > > > >a qemu instance running BMC firmware to serve as an external BMC > > for > > > > a qemu > > > > >instance running server software. It utilizes the KCS module we > > > > implemented. > > > > > > > > Looking at the IPMI changes, why didn't you just re-use the existing > > > > IPMI infrastructure? ipmi_host.[ch] is basically a subset of > > ipmi.[ch], > > > > and the ipmi_host_extern looks like a copy of of ipmi_bmc_extern with > > > > some names changed. That kind of code d
Re: [PULL 0/6] s390x update
On Fri, 11 Dec 2020 at 12:27, Cornelia Huck wrote: > > The following changes since commit 2ecfc0657afa5d29a373271b342f704a1a3c6737: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2020-12-10' > into staging (2020-12-10 17:01:05 +) > > are available in the Git repository at: > > https://github.com/cohuck/qemu tags/s390x-20201211 > > for you to fetch changes up to c7454f05171405b8013a9d6b57045cd614ccc386: > > s390x/cpu: Use timer_free() in the finalize function to avoid memleaks > (2020-12-11 11:38:10 +0100) > > > First set of 6.0 patches for s390x: > - acceptance test for device detection > - bugfixes > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0 for any user-visible changes. -- PMM
Re: [PATCH 0/7] Additional NPCM7xx devices and IPMI BMC emulation support
Thanks for the comments! I've removed IPMI part from the patch sets. I'll send a separate patch sets once the refactor is done. I'll also include Havard's documentation in it. I haven't thought of a better name. We can update the name accordingly. On Fri, Dec 11, 2020 at 4:26 PM Havard Skinnemoen wrote: > On Fri, Dec 11, 2020 at 4:16 PM Corey Minyard wrote: > >> On Fri, Dec 11, 2020 at 12:56:07PM -0800, Hao Wu wrote: >> > Tl,dr: We'll remove the IPMI changes from the current patch set and >> > refactor >> > them in a separate patch set. >> > >> > Thank you for your review! On high level, we are trying to emulate the >> BMC >> > side of the IPMI protocol. So we cannot directly use the existing IPMI >> code. >> > However, they do have a lot in duplication as you pointed out. So we'll >> > refactor >> > the existing IPMI code and update in a way that we only add the required >> > functionality. >> >> Ah, I didn't figure that out from what you posted. So the idea is you >> can create the BMC side of the system in one qemu session with your >> changes and then you connect it to a host system running qemu with the >> host side of the interface. >> >> The wire protocol is basically symmetric, but the command handling will >> need to be separate. So you probably want to split out the base >> protocol from ipmi_bmc_extern into its own file and use that from your >> own file, to avoid the duplication. >> >> You need to do proper ATTN handling on the BMC side. And you will also >> need ties into GPIOs and whatnot for doing the reset, NMI, etc. >> >> "ipmi_host" is probably not the best name. At least to me that implied >> the host side of the interface. I'm not coming up with something I >> really like, though. Maybe "bmc_host"? That's more descriptive, though >> I'm sure a better name exists. Then you could have "bmc_host_extern" >> for the protocol. If you come up with a better naming scheme, the >> existing files can be renamed, too. >> > > The naming is my fault. > > My thinking was that ipmi-host-extern is to the BMC what ipmi-bmc-extern > is to the host. That is, the former represents the host as seen by the BMC, > and the latter represents the BMC as seen by the host. > > I sent some docs to the list earlier this year, but sadly, I never got > around to follow up. You can see the generated docs here: > > https://hskinnemoen.github.io/qemu/specs/ipmi.html > > Hao, perhaps you should include my documentation patches in your next IPMI > series? If we come up with a better naming scheme for both sides, I can > update the docs accordingly. > > Havard > > >> Thanks, >> >> -corey >> >> > >> > As for the KCS module, the BMC side of the protocol is the opposite >> > direction >> > of the existing ipmi_kcs.c code which is on the host/CPU side. For >> example, >> > in READ_STATE the CPU would read data while the BMC would write data. >> > So we can't directly use the same implementation. (They're different >> files >> > in >> > Linux either.) However, we can refactor it to re-use some of the common >> > definitions. >> > >> > We would like to remove the IPMI and KCS stuff from the current patch >> set. >> > We'll send the refactored code in a separate patch set after addressing >> > your concerns. >> > >> > Thanks again for the review! >> > >> > On Thu, Dec 10, 2020 at 7:04 PM Corey Minyard wrote: >> > >> > > On Thu, Dec 10, 2020 at 05:51:49PM -0800, Hao Wu wrote: >> > > > This patch series include a few more NPCM7XX devices including >> > > > >> > > > - Analog Digital Converter (ADC) >> > > > - Pulse Width Modulation (PWM) >> > > > - Keyboard Style Controller (KSC) >> > > > >> > > > To utilize these modules we also add two extra functionalities: >> > > > >> > > > 1. We modified the CLK module to generate clock values using >> qdev_clock. >> > > >These clocks are used to determine various clocks in NPCM7XX >> devices. >> > > > 2. We added support for emulating IPMI responder devices in BMC >> machines, >> > > >similar to the existing IPMI device support for CPU emulation. >> This >> > > allows >> > > >a qemu instance running BMC firmware to serve as an external BMC >> for >> > > a qemu >> > > >instance running server software. It utilizes the KCS module we >> > > implemented. >> > > >> > > Looking at the IPMI changes, why didn't you just re-use the existing >> > > IPMI infrastructure? ipmi_host.[ch] is basically a subset of >> ipmi.[ch], >> > > and the ipmi_host_extern looks like a copy of of ipmi_bmc_extern with >> > > some names changed. That kind of code duplication is not acceptable. >> > > Plus you copied my code and removed my copyrights, which is really >> > > not acceptable and illegal. >> > > >> > > I'm not exactly sure why you needed you own KCS interface, either. It >> > > looks like the interface is somewhat different in some ways, but >> > > integrating it into the current KCS code is probably a better choice >> if >> > > that can be done. >> > > >> > > -corey >>
Re: [PATCH 0/7] Additional NPCM7xx devices and IPMI BMC emulation support
On Fri, Dec 11, 2020 at 12:56:07PM -0800, Hao Wu wrote: > Tl,dr: We'll remove the IPMI changes from the current patch set and > refactor > them in a separate patch set. > > Thank you for your review! On high level, we are trying to emulate the BMC > side of the IPMI protocol. So we cannot directly use the existing IPMI code. > However, they do have a lot in duplication as you pointed out. So we'll > refactor > the existing IPMI code and update in a way that we only add the required > functionality. Ah, I didn't figure that out from what you posted. So the idea is you can create the BMC side of the system in one qemu session with your changes and then you connect it to a host system running qemu with the host side of the interface. The wire protocol is basically symmetric, but the command handling will need to be separate. So you probably want to split out the base protocol from ipmi_bmc_extern into its own file and use that from your own file, to avoid the duplication. You need to do proper ATTN handling on the BMC side. And you will also need ties into GPIOs and whatnot for doing the reset, NMI, etc. "ipmi_host" is probably not the best name. At least to me that implied the host side of the interface. I'm not coming up with something I really like, though. Maybe "bmc_host"? That's more descriptive, though I'm sure a better name exists. Then you could have "bmc_host_extern" for the protocol. If you come up with a better naming scheme, the existing files can be renamed, too. Thanks, -corey > > As for the KCS module, the BMC side of the protocol is the opposite > direction > of the existing ipmi_kcs.c code which is on the host/CPU side. For example, > in READ_STATE the CPU would read data while the BMC would write data. > So we can't directly use the same implementation. (They're different files > in > Linux either.) However, we can refactor it to re-use some of the common > definitions. > > We would like to remove the IPMI and KCS stuff from the current patch set. > We'll send the refactored code in a separate patch set after addressing > your concerns. > > Thanks again for the review! > > On Thu, Dec 10, 2020 at 7:04 PM Corey Minyard wrote: > > > On Thu, Dec 10, 2020 at 05:51:49PM -0800, Hao Wu wrote: > > > This patch series include a few more NPCM7XX devices including > > > > > > - Analog Digital Converter (ADC) > > > - Pulse Width Modulation (PWM) > > > - Keyboard Style Controller (KSC) > > > > > > To utilize these modules we also add two extra functionalities: > > > > > > 1. We modified the CLK module to generate clock values using qdev_clock. > > >These clocks are used to determine various clocks in NPCM7XX devices. > > > 2. We added support for emulating IPMI responder devices in BMC machines, > > >similar to the existing IPMI device support for CPU emulation. This > > allows > > >a qemu instance running BMC firmware to serve as an external BMC for > > a qemu > > >instance running server software. It utilizes the KCS module we > > implemented. > > > > Looking at the IPMI changes, why didn't you just re-use the existing > > IPMI infrastructure? ipmi_host.[ch] is basically a subset of ipmi.[ch], > > and the ipmi_host_extern looks like a copy of of ipmi_bmc_extern with > > some names changed. That kind of code duplication is not acceptable. > > Plus you copied my code and removed my copyrights, which is really > > not acceptable and illegal. > > > > I'm not exactly sure why you needed you own KCS interface, either. It > > looks like the interface is somewhat different in some ways, but > > integrating it into the current KCS code is probably a better choice if > > that can be done. > > > > -corey > > > > > > > > Hao Wu (7): > > > hw/misc: Add clock converter in NPCM7XX CLK module > > > hw/timer: Refactor NPCM7XX Timer to use CLK clock > > > hw/adc: Add an ADC module for NPCM7XX > > > hw/misc: Add a PWM module for NPCM7XX > > > hw/ipmi: Add an IPMI host interface > > > hw/ipmi: Add a KCS Module for NPCM7XX > > > hw/ipmi: Add an IPMI external host device > > > > > > default-configs/devices/arm-softmmu.mak | 2 + > > > docs/system/arm/nuvoton.rst | 6 +- > > > hw/adc/meson.build | 1 + > > > hw/adc/npcm7xx_adc.c| 318 ++ > > > hw/arm/npcm7xx.c| 65 +- > > > hw/ipmi/Kconfig | 5 + > > > hw/ipmi/ipmi_host.c | 40 ++ > > > hw/ipmi/ipmi_host_extern.c | 435 + > > > hw/ipmi/meson.build | 3 + > > > hw/ipmi/npcm7xx_kcs.c | 570 + > > > hw/misc/meson.build | 1 + > > > hw/misc/npcm7xx_clk.c | 795 +++- > > > hw/misc/npcm7xx_pwm.c | 535 > > > hw/timer/npcm7xx_timer.c| 25 +- > > > include/hw/adc/npcm7xx_adc.h|
Re: [PATCH 0/7] Additional NPCM7xx devices and IPMI BMC emulation support
On Fri, Dec 11, 2020 at 4:16 PM Corey Minyard wrote: > On Fri, Dec 11, 2020 at 12:56:07PM -0800, Hao Wu wrote: > > Tl,dr: We'll remove the IPMI changes from the current patch set and > > refactor > > them in a separate patch set. > > > > Thank you for your review! On high level, we are trying to emulate the > BMC > > side of the IPMI protocol. So we cannot directly use the existing IPMI > code. > > However, they do have a lot in duplication as you pointed out. So we'll > > refactor > > the existing IPMI code and update in a way that we only add the required > > functionality. > > Ah, I didn't figure that out from what you posted. So the idea is you > can create the BMC side of the system in one qemu session with your > changes and then you connect it to a host system running qemu with the > host side of the interface. > > The wire protocol is basically symmetric, but the command handling will > need to be separate. So you probably want to split out the base > protocol from ipmi_bmc_extern into its own file and use that from your > own file, to avoid the duplication. > > You need to do proper ATTN handling on the BMC side. And you will also > need ties into GPIOs and whatnot for doing the reset, NMI, etc. > > "ipmi_host" is probably not the best name. At least to me that implied > the host side of the interface. I'm not coming up with something I > really like, though. Maybe "bmc_host"? That's more descriptive, though > I'm sure a better name exists. Then you could have "bmc_host_extern" > for the protocol. If you come up with a better naming scheme, the > existing files can be renamed, too. > The naming is my fault. My thinking was that ipmi-host-extern is to the BMC what ipmi-bmc-extern is to the host. That is, the former represents the host as seen by the BMC, and the latter represents the BMC as seen by the host. I sent some docs to the list earlier this year, but sadly, I never got around to follow up. You can see the generated docs here: https://hskinnemoen.github.io/qemu/specs/ipmi.html Hao, perhaps you should include my documentation patches in your next IPMI series? If we come up with a better naming scheme for both sides, I can update the docs accordingly. Havard > Thanks, > > -corey > > > > > As for the KCS module, the BMC side of the protocol is the opposite > > direction > > of the existing ipmi_kcs.c code which is on the host/CPU side. For > example, > > in READ_STATE the CPU would read data while the BMC would write data. > > So we can't directly use the same implementation. (They're different > files > > in > > Linux either.) However, we can refactor it to re-use some of the common > > definitions. > > > > We would like to remove the IPMI and KCS stuff from the current patch > set. > > We'll send the refactored code in a separate patch set after addressing > > your concerns. > > > > Thanks again for the review! > > > > On Thu, Dec 10, 2020 at 7:04 PM Corey Minyard wrote: > > > > > On Thu, Dec 10, 2020 at 05:51:49PM -0800, Hao Wu wrote: > > > > This patch series include a few more NPCM7XX devices including > > > > > > > > - Analog Digital Converter (ADC) > > > > - Pulse Width Modulation (PWM) > > > > - Keyboard Style Controller (KSC) > > > > > > > > To utilize these modules we also add two extra functionalities: > > > > > > > > 1. We modified the CLK module to generate clock values using > qdev_clock. > > > >These clocks are used to determine various clocks in NPCM7XX > devices. > > > > 2. We added support for emulating IPMI responder devices in BMC > machines, > > > >similar to the existing IPMI device support for CPU emulation. > This > > > allows > > > >a qemu instance running BMC firmware to serve as an external BMC > for > > > a qemu > > > >instance running server software. It utilizes the KCS module we > > > implemented. > > > > > > Looking at the IPMI changes, why didn't you just re-use the existing > > > IPMI infrastructure? ipmi_host.[ch] is basically a subset of > ipmi.[ch], > > > and the ipmi_host_extern looks like a copy of of ipmi_bmc_extern with > > > some names changed. That kind of code duplication is not acceptable. > > > Plus you copied my code and removed my copyrights, which is really > > > not acceptable and illegal. > > > > > > I'm not exactly sure why you needed you own KCS interface, either. It > > > looks like the interface is somewhat different in some ways, but > > > integrating it into the current KCS code is probably a better choice if > > > that can be done. > > > > > > -corey > > > > > > > > > > > Hao Wu (7): > > > > hw/misc: Add clock converter in NPCM7XX CLK module > > > > hw/timer: Refactor NPCM7XX Timer to use CLK clock > > > > hw/adc: Add an ADC module for NPCM7XX > > > > hw/misc: Add a PWM module for NPCM7XX > > > > hw/ipmi: Add an IPMI host interface > > > > hw/ipmi: Add a KCS Module for NPCM7XX > > > > hw/ipmi: Add an IPMI external host device > > > > > > > > default-configs/devices/arm-soft
[PATCH 7/8] hw/ppc/ppc405_uc: Drop use of ppcuic_init()
Switch the ppc405_uc boards to directly creating and configuring the
UIC, rather than doing it via the old ppcuic_init() helper function.
We retain the API feature of ppc405ep_init() where it passes back
something allowing the callers to wire up devices to the UIC if
they need to, even though neither of the callsites currently makes
use of this ability -- instead of passing back the qemu_irq array
we pass back the UIC DeviceState.
This fixes a trivial Coverity-detected memory leak where
we were leaking the array of IRQs returned by ppcuic_init().
Fixes: Coverity CID 1421922
Signed-off-by: Peter Maydell
---
hw/ppc/ppc405.h| 2 +-
hw/ppc/ppc405_boards.c | 8 ++---
hw/ppc/ppc405_uc.c | 70 +-
3 files changed, 47 insertions(+), 33 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index e6c702f7e0d..c58f739886a 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -66,7 +66,7 @@ CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
MemoryRegion ram_memories[2],
hwaddr ram_bases[2],
hwaddr ram_sizes[2],
-uint32_t sysclk, qemu_irq **picp,
+uint32_t sysclk, DeviceState **uicdev,
int do_init);
#endif /* PPC405_H */
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index b7249f21cf2..8f77887fb18 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -151,7 +151,6 @@ static void ref405ep_init(MachineState *machine)
CPUPPCState *env;
DeviceState *dev;
SysBusDevice *s;
-qemu_irq *pic;
MemoryRegion *bios;
MemoryRegion *sram = g_new(MemoryRegion, 1);
ram_addr_t bdloc;
@@ -167,6 +166,7 @@ static void ref405ep_init(MachineState *machine)
int len;
DriveInfo *dinfo;
MemoryRegion *sysmem = get_system_memory();
+DeviceState *uicdev;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -184,7 +184,7 @@ static void ref405ep_init(MachineState *machine)
ram_bases[1] = 0x;
ram_sizes[1] = 0x;
env = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
-, &pic, kernel_filename == NULL ? 0 : 1);
+, &uicdev, kernel_filename == NULL ? 0 : 1);
/* allocate SRAM */
sram_size = 512 * KiB;
memory_region_init_ram(sram, NULL, "ef405ep.sram", sram_size,
@@ -429,7 +429,6 @@ static void taihu_405ep_init(MachineState *machine)
const char *kernel_filename = machine->kernel_filename;
const char *initrd_filename = machine->initrd_filename;
char *filename;
-qemu_irq *pic;
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *bios;
MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
@@ -440,6 +439,7 @@ static void taihu_405ep_init(MachineState *machine)
int linux_boot;
int fl_idx;
DriveInfo *dinfo;
+DeviceState *uicdev;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -459,7 +459,7 @@ static void taihu_405ep_init(MachineState *machine)
"taihu_405ep.ram-1", machine->ram, ram_bases[1],
ram_sizes[1]);
ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
- , &pic, kernel_filename == NULL ? 0 : 1);
+ , &uicdev, kernel_filename == NULL ? 0 : 1);
/* allocate and load BIOS */
fl_idx = 0;
#if defined(USE_FLASH_BIOS)
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 3e191ae4af5..fe047074a17 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -36,6 +36,9 @@
#include "sysemu/sysemu.h"
#include "qemu/log.h"
#include "exec/address-spaces.h"
+#include "hw/intc/ppc-uic.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
//#define DEBUG_OPBA
//#define DEBUG_SDRAM
@@ -1446,14 +1449,15 @@ CPUPPCState *ppc405ep_init(MemoryRegion
*address_space_mem,
MemoryRegion ram_memories[2],
hwaddr ram_bases[2],
hwaddr ram_sizes[2],
-uint32_t sysclk, qemu_irq **picp,
+uint32_t sysclk, DeviceState **uicdevp,
int do_init)
{
clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
PowerPCCPU *cpu;
CPUPPCState *env;
-qemu_irq *pic, *irqs;
+DeviceState *uicdev;
+SysBusDevice *uicsbd;
memset(clk_setup, 0, sizeof(clk_setup));
/* init CPUs */
@@ -1474,59 +1478,69 @@ CPUPPCState *ppc405ep_init(MemoryRegion
*address_space_mem,
/* Initialize timers */
ppc_booke_timers_init(cpu, sysclk, 0);
/* Universal interrupt controller */
-irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-irqs[PPCUIC_OUTP
[PATCH 8/8] hw/ppc: Remove unused ppcuic_init()
Now we've converted all the callsites to directly create the QOM UIC
device themselves, the ppcuic_init() function is unused and can be
removed. The enum defining PPCUIC symbolic constants can be moved
to the ppc-uic.h header where it more naturally belongs.
Signed-off-by: Peter Maydell
---
include/hw/intc/ppc-uic.h | 7 +++
include/hw/ppc/ppc4xx.h | 9 -
hw/ppc/ppc4xx_devs.c | 38 --
3 files changed, 7 insertions(+), 47 deletions(-)
diff --git a/include/hw/intc/ppc-uic.h b/include/hw/intc/ppc-uic.h
index e614e2ffd80..22dd5e5ac2c 100644
--- a/include/hw/intc/ppc-uic.h
+++ b/include/hw/intc/ppc-uic.h
@@ -47,6 +47,13 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPCUIC, PPC_UIC)
#define UIC_MAX_IRQ 32
+/* Symbolic constants for the sysbus IRQ outputs */
+enum {
+PPCUIC_OUTPUT_INT = 0,
+PPCUIC_OUTPUT_CINT = 1,
+PPCUIC_OUTPUT_NB,
+};
+
struct PPCUIC {
/*< private >*/
SysBusDevice parent_obj;
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index cc19c8da5be..980f964b5a9 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -33,15 +33,6 @@ PowerPCCPU *ppc4xx_init(const char *cpu_model,
clk_setup_t *cpu_clk, clk_setup_t *tb_clk,
uint32_t sysclk);
-/* PowerPC 4xx universal interrupt controller */
-enum {
-PPCUIC_OUTPUT_INT = 0,
-PPCUIC_OUTPUT_CINT = 1,
-PPCUIC_OUTPUT_NB,
-};
-qemu_irq *ppcuic_init (CPUPPCState *env, qemu_irq *irqs,
- uint32_t dcr_base, int has_ssr, int has_vr);
-
void ppc4xx_sdram_banks(MemoryRegion *ram, int nr_banks,
MemoryRegion ram_memories[],
hwaddr ram_bases[], hwaddr ram_sizes[],
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index ffe4cf43e88..fe9d4f7155e 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -77,44 +77,6 @@ PowerPCCPU *ppc4xx_init(const char *cpu_type,
return cpu;
}
-/*/
-/* "Universal" Interrupt controller */
-
-qemu_irq *ppcuic_init (CPUPPCState *env, qemu_irq *irqs,
- uint32_t dcr_base, int has_ssr, int has_vr)
-{
-DeviceState *uicdev = qdev_new(TYPE_PPC_UIC);
-SysBusDevice *uicsbd = SYS_BUS_DEVICE(uicdev);
-qemu_irq *uic_irqs;
-int i;
-
-qdev_prop_set_uint32(uicdev, "dcr-base", dcr_base);
-qdev_prop_set_bit(uicdev, "use-vectors", has_vr);
-object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(env_cpu(env)),
- &error_fatal);
-sysbus_realize_and_unref(uicsbd, &error_fatal);
-
-sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT, irqs[PPCUIC_OUTPUT_INT]);
-sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT, irqs[PPCUIC_OUTPUT_CINT]);
-
-/*
- * Return an allocated array of the UIC's input IRQ lines.
- * This is an ugly temporary API to retain compatibility with
- * the ppcuic_init() interface from the pre-QOM-conversion UIC.
- * None of the callers free this array, so it is leaked -- but
- * so was the array allocated by qemu_allocate_irqs() in the
- * old code.
- *
- * The callers should just instantiate the UIC and wire it up
- * themselves rather than passing qemu_irq* in and out of this function.
- */
-uic_irqs = g_new0(qemu_irq, UIC_MAX_IRQ);
-for (i = 0; i < UIC_MAX_IRQ; i++) {
-uic_irqs[i] = qdev_get_gpio_in(uicdev, i);
-}
-return uic_irqs;
-}
-
/*/
/* SDRAM controller */
typedef struct ppc4xx_sdram_t ppc4xx_sdram_t;
--
2.20.1
[PATCH 6/8] hw/ppc: Delete unused ppc405cr_init() code
The function ppc405cr_init() has apparently been unused since it was
added in commit 8ecc7913525ecb in 2007.
Remove this dead code, so we don't have to convert it away from using
ppcuic_init().
Signed-off-by: Peter Maydell
---
hw/ppc/ppc405.h| 6 -
hw/ppc/ppc405_uc.c | 345 -
2 files changed, 351 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 7ed25cfa1bf..e6c702f7e0d 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -62,12 +62,6 @@ ram_addr_t ppc405_set_bootinfo (CPUPPCState *env,
ppc4xx_bd_info_t *bd,
void ppc4xx_plb_init(CPUPPCState *env);
void ppc405_ebc_init(CPUPPCState *env);
-CPUPPCState *ppc405cr_init(MemoryRegion *address_space_mem,
-MemoryRegion ram_memories[4],
-hwaddr ram_bases[4],
-hwaddr ram_sizes[4],
-uint32_t sysclk, qemu_irq **picp,
-int do_init);
CPUPPCState *ppc405ep_init(MemoryRegion *address_space_mem,
MemoryRegion ram_memories[2],
hwaddr ram_bases[2],
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 381720aced9..3e191ae4af5 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1155,351 +1155,6 @@ static void ppc4xx_gpt_init(hwaddr base, qemu_irq
irqs[5])
qemu_register_reset(ppc4xx_gpt_reset, gpt);
}
-/*/
-/* PowerPC 405CR */
-enum {
-PPC405CR_CPC0_PLLMR = 0x0B0,
-PPC405CR_CPC0_CR0= 0x0B1,
-PPC405CR_CPC0_CR1= 0x0B2,
-PPC405CR_CPC0_PSR= 0x0B4,
-PPC405CR_CPC0_JTAGID = 0x0B5,
-PPC405CR_CPC0_ER = 0x0B9,
-PPC405CR_CPC0_FR = 0x0BA,
-PPC405CR_CPC0_SR = 0x0BB,
-};
-
-enum {
-PPC405CR_CPU_CLK = 0,
-PPC405CR_TMR_CLK = 1,
-PPC405CR_PLB_CLK = 2,
-PPC405CR_SDRAM_CLK = 3,
-PPC405CR_OPB_CLK = 4,
-PPC405CR_EXT_CLK = 5,
-PPC405CR_UART_CLK = 6,
-PPC405CR_CLK_NB= 7,
-};
-
-typedef struct ppc405cr_cpc_t ppc405cr_cpc_t;
-struct ppc405cr_cpc_t {
-clk_setup_t clk_setup[PPC405CR_CLK_NB];
-uint32_t sysclk;
-uint32_t psr;
-uint32_t cr0;
-uint32_t cr1;
-uint32_t jtagid;
-uint32_t pllmr;
-uint32_t er;
-uint32_t fr;
-};
-
-static void ppc405cr_clk_setup (ppc405cr_cpc_t *cpc)
-{
-uint64_t VCO_out, PLL_out;
-uint32_t CPU_clk, TMR_clk, SDRAM_clk, PLB_clk, OPB_clk, EXT_clk, UART_clk;
-int M, D0, D1, D2;
-
-D0 = ((cpc->pllmr >> 26) & 0x3) + 1; /* CBDV */
-if (cpc->pllmr & 0x8000) {
-D1 = (((cpc->pllmr >> 20) - 1) & 0xF) + 1; /* FBDV */
-D2 = 8 - ((cpc->pllmr >> 16) & 0x7); /* FWDVA */
-M = D0 * D1 * D2;
-VCO_out = (uint64_t)cpc->sysclk * M;
-if (VCO_out < 4 || VCO_out > 8) {
-/* PLL cannot lock */
-cpc->pllmr &= ~0x8000;
-goto bypass_pll;
-}
-PLL_out = VCO_out / D2;
-} else {
-/* Bypass PLL */
-bypass_pll:
-M = D0;
-PLL_out = (uint64_t)cpc->sysclk * M;
-}
-CPU_clk = PLL_out;
-if (cpc->cr1 & 0x0080)
-TMR_clk = cpc->sysclk; /* Should have a separate clock */
-else
-TMR_clk = CPU_clk;
-PLB_clk = CPU_clk / D0;
-SDRAM_clk = PLB_clk;
-D0 = ((cpc->pllmr >> 10) & 0x3) + 1;
-OPB_clk = PLB_clk / D0;
-D0 = ((cpc->pllmr >> 24) & 0x3) + 2;
-EXT_clk = PLB_clk / D0;
-D0 = ((cpc->cr0 >> 1) & 0x1F) + 1;
-UART_clk = CPU_clk / D0;
-/* Setup CPU clocks */
-clk_setup(&cpc->clk_setup[PPC405CR_CPU_CLK], CPU_clk);
-/* Setup time-base clock */
-clk_setup(&cpc->clk_setup[PPC405CR_TMR_CLK], TMR_clk);
-/* Setup PLB clock */
-clk_setup(&cpc->clk_setup[PPC405CR_PLB_CLK], PLB_clk);
-/* Setup SDRAM clock */
-clk_setup(&cpc->clk_setup[PPC405CR_SDRAM_CLK], SDRAM_clk);
-/* Setup OPB clock */
-clk_setup(&cpc->clk_setup[PPC405CR_OPB_CLK], OPB_clk);
-/* Setup external clock */
-clk_setup(&cpc->clk_setup[PPC405CR_EXT_CLK], EXT_clk);
-/* Setup UART clock */
-clk_setup(&cpc->clk_setup[PPC405CR_UART_CLK], UART_clk);
-}
-
-static uint32_t dcr_read_crcpc (void *opaque, int dcrn)
-{
-ppc405cr_cpc_t *cpc;
-uint32_t ret;
-
-cpc = opaque;
-switch (dcrn) {
-case PPC405CR_CPC0_PLLMR:
-ret = cpc->pllmr;
-break;
-case PPC405CR_CPC0_CR0:
-ret = cpc->cr0;
-break;
-case PPC405CR_CPC0_CR1:
-ret = cpc->cr1;
-break;
-case PPC405CR_CPC0_PSR:
-ret = cpc->psr;
-break;
-case PPC405CR_CPC0_JTAGID:
-ret = cpc->jtagid;
-break;
-case PPC405CR_CPC0_ER:
-ret = cpc->er;
-break;
-case PPC405CR_CPC0_FR:
-ret = cpc->fr;
-break;
-case PPC405CR_CPC0_SR:
-ret = ~(cpc->er | cpc->fr) & 0x;
-break;
-default:
-/* Avoid gcc warning *
[PATCH 5/8] hw/ppc/sam460ex: Drop use of ppcuic_init()
Switch the sam460ex board to directly creating and configuring the
UIC, rather than doing it via the old ppcuic_init() helper function.
Signed-off-by: Peter Maydell
---
hw/ppc/sam460ex.c | 70 ---
1 file changed, 54 insertions(+), 16 deletions(-)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 14e6583eb0d..9cf7aad3833 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -39,6 +39,7 @@
#include "hw/usb/hcd-ehci.h"
#include "hw/ppc/fdt.h"
#include "hw/qdev-properties.h"
+#include "hw/intc/ppc-uic.h"
#include
@@ -281,7 +282,6 @@ static void sam460ex_init(MachineState *machine)
hwaddr ram_bases[SDRAM_NR_BANKS] = {0};
hwaddr ram_sizes[SDRAM_NR_BANKS] = {0};
MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
-qemu_irq *irqs, *uic[4];
PCIBus *pci_bus;
PowerPCCPU *cpu;
CPUPPCState *env;
@@ -293,6 +293,9 @@ static void sam460ex_init(MachineState *machine)
struct boot_info *boot_info;
uint8_t *spd_data;
int success;
+qemu_irq mal_irqs[4];
+DeviceState *uic[4];
+int i;
cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
env = &cpu->env;
@@ -312,13 +315,35 @@ static void sam460ex_init(MachineState *machine)
ppc4xx_plb_init(env);
/* interrupt controllers */
-irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq
*)env->irq_inputs)[PPC40x_INPUT_CINT];
-uic[0] = ppcuic_init(env, irqs, 0xc0, 0, 1);
-uic[1] = ppcuic_init(env, &uic[0][30], 0xd0, 0, 1);
-uic[2] = ppcuic_init(env, &uic[0][10], 0xe0, 0, 1);
-uic[3] = ppcuic_init(env, &uic[0][16], 0xf0, 0, 1);
+for (i = 0; i < ARRAY_SIZE(uic); i++) {
+SysBusDevice *sbd;
+/*
+ * Number of the first of the two consecutive IRQ inputs on UIC 0
+ * to connect the INT and CINT outputs of UIC n to. The entry
+ * for UIC 0 is ignored, because it connects to the CPU.
+ */
+const int input_ints[] = { -1, 30, 10, 16 };
+
+uic[i] = qdev_new(TYPE_PPC_UIC);
+sbd = SYS_BUS_DEVICE(uic[i]);
+
+qdev_prop_set_uint32(uic[i], "dcr-base", 0xc0 + i * 0x10);
+object_property_set_link(OBJECT(uic[i]), "cpu", OBJECT(cpu),
+ &error_fatal);
+sysbus_realize_and_unref(sbd, &error_fatal);
+
+if (i == 0) {
+sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
+ ((qemu_irq
*)env->irq_inputs)[PPC40x_INPUT_INT]);
+sysbus_connect_irq(sbd, PPCUIC_OUTPUT_CINT,
+ ((qemu_irq
*)env->irq_inputs)[PPC40x_INPUT_CINT]);
+} else {
+sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
+ qdev_get_gpio_in(uic[0], input_ints[i]));
+sysbus_connect_irq(sbd, PPCUIC_OUTPUT_INT,
+ qdev_get_gpio_in(uic[0], input_ints[i] + 1));
+}
+}
/* SDRAM controller */
/* put all RAM on first bank because board has one slot
@@ -331,7 +356,8 @@ static void sam460ex_init(MachineState *machine)
ram_bases, ram_sizes, 1);
/* IIC controllers and devices */
-dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700, uic[0][2]);
+dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600700,
+ qdev_get_gpio_in(uic[0], 2));
i2c = PPC4xx_I2C(dev)->bus;
/* SPD EEPROM on RAM module */
spd_data = spd_data_generate(ram_sizes[0] < 128 * MiB ? DDR : DDR2,
@@ -341,7 +367,8 @@ static void sam460ex_init(MachineState *machine)
/* RTC */
i2c_slave_create_simple(i2c, "m41t80", 0x68);
-dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800, uic[0][3]);
+dev = sysbus_create_simple(TYPE_PPC4xx_I2C, 0x4ef600800,
+ qdev_get_gpio_in(uic[0], 3));
/* External bus controller */
ppc405_ebc_init(env);
@@ -356,7 +383,14 @@ static void sam460ex_init(MachineState *machine)
ppc4xx_sdr_init(env);
/* MAL */
-ppc4xx_mal_init(env, 4, 16, &uic[2][3]);
+/*
+ * TODO if the MAL were a proper QOM device we would not need to
+ * copy its qemu_irqs into an array for ppc4xx_mal_init()'s benefit.
+ */
+for (i = 0; i < ARRAY_SIZE(mal_irqs); i++) {
+mal_irqs[0] = qdev_get_gpio_in(uic[2], 3 + i);
+}
+ppc4xx_mal_init(env, 4, 16, mal_irqs);
/* DMA */
ppc4xx_dma_init(env, 0x200);
@@ -369,21 +403,23 @@ static void sam460ex_init(MachineState *machine)
memory_region_add_subregion(address_space_mem, 0x4LL, l2cache_ram);
/* USB */
-sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
+sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400,
+ qdev_get_gpio_in(uic[2], 29));
dev = qdev_new("sysbus-ohci");
qdev_prop_set_string(dev, "masterbus", "us
[PATCH 4/8] hw/ppc/ppc440_bamboo: Drop use of ppcuic_init()
Switch the bamboo board to directly creating and configuring the UIC,
rather than doing it via the old ppcuic_init() helper function.
Signed-off-by: Peter Maydell
---
hw/ppc/ppc440_bamboo.c | 38 +++---
1 file changed, 27 insertions(+), 11 deletions(-)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 665bc1784e1..b156bcb9990 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -33,6 +33,9 @@
#include "sysemu/qtest.h"
#include "sysemu/reset.h"
#include "hw/sysbus.h"
+#include "hw/intc/ppc-uic.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
#define BINARY_DEVICE_TREE_FILE "bamboo.dtb"
@@ -168,13 +171,13 @@ static void bamboo_init(MachineState *machine)
MemoryRegion *ram_memories = g_new(MemoryRegion, PPC440EP_SDRAM_NR_BANKS);
hwaddr ram_bases[PPC440EP_SDRAM_NR_BANKS];
hwaddr ram_sizes[PPC440EP_SDRAM_NR_BANKS];
-qemu_irq *pic;
-qemu_irq *irqs;
PCIBus *pcibus;
PowerPCCPU *cpu;
CPUPPCState *env;
target_long initrd_size = 0;
DeviceState *dev;
+DeviceState *uicdev;
+SysBusDevice *uicsbd;
int success;
int i;
@@ -192,10 +195,17 @@ static void bamboo_init(MachineState *machine)
ppc_dcr_init(env, NULL, NULL);
/* interrupt controller */
-irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq
*)env->irq_inputs)[PPC40x_INPUT_CINT];
-pic = ppcuic_init(env, irqs, 0x0C0, 0, 1);
+uicdev = qdev_new(TYPE_PPC_UIC);
+uicsbd = SYS_BUS_DEVICE(uicdev);
+
+object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
+ &error_fatal);
+sysbus_realize_and_unref(uicsbd, &error_fatal);
+
+sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
+ ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
+sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
+ ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
/* SDRAM controller */
memset(ram_bases, 0, sizeof(ram_bases));
@@ -203,14 +213,18 @@ static void bamboo_init(MachineState *machine)
ppc4xx_sdram_banks(machine->ram, PPC440EP_SDRAM_NR_BANKS, ram_memories,
ram_bases, ram_sizes, ppc440ep_sdram_bank_sizes);
/* XXX 440EP's ECC interrupts are on UIC1, but we've only created UIC0. */
-ppc4xx_sdram_init(env, pic[14], PPC440EP_SDRAM_NR_BANKS, ram_memories,
+ppc4xx_sdram_init(env,
+ qdev_get_gpio_in(uicdev, 14),
+ PPC440EP_SDRAM_NR_BANKS, ram_memories,
ram_bases, ram_sizes, 1);
/* PCI */
dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST_BRIDGE,
PPC440EP_PCI_CONFIG,
-pic[pci_irq_nrs[0]], pic[pci_irq_nrs[1]],
-pic[pci_irq_nrs[2]], pic[pci_irq_nrs[3]],
+qdev_get_gpio_in(uicdev, pci_irq_nrs[0]),
+qdev_get_gpio_in(uicdev, pci_irq_nrs[1]),
+qdev_get_gpio_in(uicdev, pci_irq_nrs[2]),
+qdev_get_gpio_in(uicdev, pci_irq_nrs[3]),
NULL);
pcibus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
if (!pcibus) {
@@ -223,12 +237,14 @@ static void bamboo_init(MachineState *machine)
memory_region_add_subregion(get_system_memory(), PPC440EP_PCI_IO, isa);
if (serial_hd(0) != NULL) {
-serial_mm_init(address_space_mem, 0xef600300, 0, pic[0],
+serial_mm_init(address_space_mem, 0xef600300, 0,
+ qdev_get_gpio_in(uicdev, 0),
PPC_SERIAL_MM_BAUDBASE, serial_hd(0),
DEVICE_BIG_ENDIAN);
}
if (serial_hd(1) != NULL) {
-serial_mm_init(address_space_mem, 0xef600400, 0, pic[1],
+serial_mm_init(address_space_mem, 0xef600400, 0,
+ qdev_get_gpio_in(uicdev, 1),
PPC_SERIAL_MM_BAUDBASE, serial_hd(1),
DEVICE_BIG_ENDIAN);
}
--
2.20.1
[PATCH 1/8] hw/ppc/ppc4xx_devs: Make code style fixes to UIC code
In a following commit we will move the PPC UIC implementation to
its own file in hw/intc. To prevent checkpatch complaining about that
code-motion, fix up the minor style issues first.
Signed-off-by: Peter Maydell
---
hw/ppc/ppc4xx_devs.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index f1651e04d9a..f2f9ca4ffec 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -105,7 +105,7 @@ struct ppcuic_t {
qemu_irq *irqs;
};
-static void ppcuic_trigger_irq (ppcuic_t *uic)
+static void ppcuic_trigger_irq(ppcuic_t *uic)
{
uint32_t ir, cr;
int start, end, inc, i;
@@ -156,26 +156,28 @@ static void ppcuic_trigger_irq (ppcuic_t *uic)
}
}
-static void ppcuic_set_irq (void *opaque, int irq_num, int level)
+static void ppcuic_set_irq(void *opaque, int irq_num, int level)
{
ppcuic_t *uic;
uint32_t mask, sr;
uic = opaque;
-mask = 1U << (31-irq_num);
+mask = 1U << (31 - irq_num);
LOG_UIC("%s: irq %d level %d uicsr %08" PRIx32
" mask %08" PRIx32 " => %08" PRIx32 " %08" PRIx32 "\n",
__func__, irq_num, level,
uic->uicsr, mask, uic->uicsr & mask, level << irq_num);
-if (irq_num < 0 || irq_num > 31)
+if (irq_num < 0 || irq_num > 31) {
return;
+}
sr = uic->uicsr;
/* Update status register */
if (uic->uictr & mask) {
/* Edge sensitive interrupt */
-if (level == 1)
+if (level == 1) {
uic->uicsr |= mask;
+}
} else {
/* Level sensitive interrupt */
if (level == 1) {
@@ -188,11 +190,12 @@ static void ppcuic_set_irq (void *opaque, int irq_num,
int level)
}
LOG_UIC("%s: irq %d level %d sr %" PRIx32 " => "
"%08" PRIx32 "\n", __func__, irq_num, level, uic->uicsr, sr);
-if (sr != uic->uicsr)
+if (sr != uic->uicsr) {
ppcuic_trigger_irq(uic);
+}
}
-static uint32_t dcr_read_uic (void *opaque, int dcrn)
+static uint32_t dcr_read_uic(void *opaque, int dcrn)
{
ppcuic_t *uic;
uint32_t ret;
@@ -220,13 +223,15 @@ static uint32_t dcr_read_uic (void *opaque, int dcrn)
ret = uic->uicsr & uic->uicer;
break;
case DCR_UICVR:
-if (!uic->use_vectors)
+if (!uic->use_vectors) {
goto no_read;
+}
ret = uic->uicvr;
break;
case DCR_UICVCR:
-if (!uic->use_vectors)
+if (!uic->use_vectors) {
goto no_read;
+}
ret = uic->uicvcr;
break;
default:
@@ -238,7 +243,7 @@ static uint32_t dcr_read_uic (void *opaque, int dcrn)
return ret;
}
-static void dcr_write_uic (void *opaque, int dcrn, uint32_t val)
+static void dcr_write_uic(void *opaque, int dcrn, uint32_t val)
{
ppcuic_t *uic;
--
2.20.1
[PATCH 2/8] ppc: Convert PPC UIC to a QOM device
Currently the PPC UIC ("Universal Interrupt Controller") is implemented
as a non-QOM device in ppc4xx_devs.c. Convert it to a proper QOM device
in hw/intc.
The ppcuic_init() function is retained for the moment with its current
interface; in subsequent commits this will be tidied up to avoid the
allocation of an irq array.
This conversion adds VMState support.
It leaves the LOG_UIC() macro as-is to maximise the extent to which
this is simply code-movement rather than a rewrite (in new code it
would be better to use tracepoints).
The default property values for dcr-base and use-vectors are set to
match those use by most of our boards with a UIC.
Signed-off-by: Peter Maydell
---
include/hw/intc/ppc-uic.h | 73 +
hw/intc/ppc-uic.c | 321 ++
hw/ppc/ppc4xx_devs.c | 267 ---
MAINTAINERS | 2 +
hw/intc/Kconfig | 3 +
hw/intc/meson.build | 1 +
hw/ppc/Kconfig| 1 +
7 files changed, 431 insertions(+), 237 deletions(-)
create mode 100644 include/hw/intc/ppc-uic.h
create mode 100644 hw/intc/ppc-uic.c
diff --git a/include/hw/intc/ppc-uic.h b/include/hw/intc/ppc-uic.h
new file mode 100644
index 000..e614e2ffd80
--- /dev/null
+++ b/include/hw/intc/ppc-uic.h
@@ -0,0 +1,73 @@
+/*
+ * "Universal" Interrupt Controller for PowerPPC 4xx embedded processors
+ *
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_INTC_PPC_UIC_H
+#define HW_INTC_PPC_UIC_H
+
+#include "hw/sysbus.h"
+#include "qom/object.h"
+
+#define TYPE_PPC_UIC "ppc-uic"
+OBJECT_DECLARE_SIMPLE_TYPE(PPCUIC, PPC_UIC)
+
+/*
+ * QEMU interface:
+ * QOM property "cpu": link to the PPC CPU
+ *(no default, must be set)
+ * QOM property "dcr-base": base of the bank of DCR registers for the UIC
+ *(default 0x30)
+ * QOM property "use-vectors": true if the UIC has vector registers
+ *(default true)
+ * unnamed GPIO inputs 0..UIC_MAX_IRQ: input IRQ lines
+ * sysbus IRQs:
+ * 0 (PPCUIC_OUTPUT_INT): output INT line to the CPU
+ * 1 (PPCUIC_OUTPUT_CINT): output CINT line to the CPU
+ */
+
+#define UIC_MAX_IRQ 32
+
+struct PPCUIC {
+/*< private >*/
+SysBusDevice parent_obj;
+
+/*< public >*/
+qemu_irq output_int;
+qemu_irq output_cint;
+
+/* properties */
+CPUState *cpu;
+uint32_t dcr_base;
+bool use_vectors;
+
+uint32_t level; /* Remembers the state of level-triggered interrupts. */
+uint32_t uicsr; /* Status register */
+uint32_t uicer; /* Enable register */
+uint32_t uiccr; /* Critical register */
+uint32_t uicpr; /* Polarity register */
+uint32_t uictr; /* Triggering register */
+uint32_t uicvcr; /* Vector configuration register */
+uint32_t uicvr;
+};
+
+#endif
diff --git a/hw/intc/ppc-uic.c b/hw/intc/ppc-uic.c
new file mode 100644
index 000..b21951eea83
--- /dev/null
+++ b/hw/intc/ppc-uic.c
@@ -0,0 +1,321 @@
+/*
+ * "Universal" Interrupt Controller for PowerPPC 4xx embedded processors
+ *
+ * Copyright (c) 2007 Jocelyn Mayer
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+
[PATCH 0/8] hw/ppc: Convert UIC device to QOM
This patchseries converts the PPC UIC "Universal Interrupt Controller" to a QOM device. My main reason for doing it is that this fixes a couple of long-standing trivial Coverity issues -- the current ppcuic_init() function allocates an array of qemu_irqs which the callers then leak. (The leak is trivial because it happens once when QEMU starts.) The patchseries converts the UIC to a QOM device but initially leaves the old ppcuic_init() creation function with its old API intact. It then goes through converting the various boards that were using ppcuic_init() to instead directly create the UIC using the usual qdev APIs, so that it can delete the ppcuic_init() function entirely. The patchset includes one patch which deletes 350 lines of dead code -- the ppc405cr_init() function seems to have never been used since it was added in 2007, so rather than converting this user of ppcuic_init() it seemed more sensible to delete it. I have tested with 'make check' and 'make check-acceptance' but I don't think the latter really exercises the affected boards, which are: bamboo ref405ep sam460ex taihu virtex-ml507 I found instructions on how to boot an AROS image on sam460ex, so I have tested that: it works as well after this series as it did before (gets to "Libs/workbench.library" and stops); it does seem to successfully do things like scanning the USB bus and responding to keyboard input at the boot menu, which suggests that IRQs must be working. Side note: the 'irq_inputs' hacks in the PPC CPU I think would really benefit from conversion to being qdev gpio inputs now that CPUs are real devices. There are also a lot of non-QOM devices in this ppc4xx code if anybody is interested in working on more QOM conversions for these boards. thanks -- PMM Peter Maydell (8): hw/ppc/ppc4xx_devs: Make code style fixes to UIC code ppc: Convert PPC UIC to a QOM device hw/ppc/virtex_ml507: Drop use of ppcuic_init() hw/ppc/ppc440_bamboo: Drop use of ppcuic_init() hw/ppc/sam460ex: Drop use of ppcuic_init() hw/ppc: Delete unused ppc405cr_init() code hw/ppc/ppc405_uc: Drop use of ppcuic_init() hw/ppc: Remove unused ppcuic_init() hw/ppc/ppc405.h | 8 +- include/hw/intc/ppc-uic.h | 80 include/hw/ppc/ppc4xx.h | 9 - hw/intc/ppc-uic.c | 321 + hw/ppc/ppc405_boards.c| 8 +- hw/ppc/ppc405_uc.c| 415 -- hw/ppc/ppc440_bamboo.c| 38 +++- hw/ppc/ppc4xx_devs.c | 246 +- hw/ppc/sam460ex.c | 70 +-- hw/ppc/virtex_ml507.c | 21 +- MAINTAINERS | 2 + hw/intc/Kconfig | 3 + hw/intc/meson.build | 1 + hw/ppc/Kconfig| 1 + 14 files changed, 555 insertions(+), 668 deletions(-) create mode 100644 include/hw/intc/ppc-uic.h create mode 100644 hw/intc/ppc-uic.c -- 2.20.1
[PATCH 3/8] hw/ppc/virtex_ml507: Drop use of ppcuic_init()
Switch the virtex_ml507 board to directly creating and
configuring the UIC, rather than doing it via the old
ppcuic_init() helper function.
This fixes a trivial Coverity-detected memory leak where
we were leaking the array of IRQs returned by ppcuic_init().
Fixes: Coverity CID 1421992
Signed-off-by: Peter Maydell
---
hw/ppc/virtex_ml507.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 7f1bca928c1..34767b11cad 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -43,6 +43,7 @@
#include "qemu/option.h"
#include "exec/address-spaces.h"
+#include "hw/intc/ppc-uic.h"
#include "hw/ppc/ppc.h"
#include "hw/ppc/ppc4xx.h"
#include "hw/qdev-properties.h"
@@ -95,7 +96,8 @@ static PowerPCCPU *ppc440_init_xilinx(const char *cpu_type,
uint32_t sysclk)
{
PowerPCCPU *cpu;
CPUPPCState *env;
-qemu_irq *irqs;
+DeviceState *uicdev;
+SysBusDevice *uicsbd;
cpu = POWERPC_CPU(cpu_create(cpu_type));
env = &cpu->env;
@@ -105,10 +107,19 @@ static PowerPCCPU *ppc440_init_xilinx(const char
*cpu_type, uint32_t sysclk)
ppc_dcr_init(env, NULL, NULL);
/* interrupt controller */
-irqs = g_new0(qemu_irq, PPCUIC_OUTPUT_NB);
-irqs[PPCUIC_OUTPUT_INT] = ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT];
-irqs[PPCUIC_OUTPUT_CINT] = ((qemu_irq
*)env->irq_inputs)[PPC40x_INPUT_CINT];
-ppcuic_init(env, irqs, 0x0C0, 0, 1);
+uicdev = qdev_new(TYPE_PPC_UIC);
+uicsbd = SYS_BUS_DEVICE(uicdev);
+
+object_property_set_link(OBJECT(uicdev), "cpu", OBJECT(cpu),
+ &error_fatal);
+sysbus_realize_and_unref(uicsbd, &error_fatal);
+
+sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_INT,
+ ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_INT]);
+sysbus_connect_irq(uicsbd, PPCUIC_OUTPUT_CINT,
+ ((qemu_irq *)env->irq_inputs)[PPC40x_INPUT_CINT]);
+
+/* This board doesn't wire anything up to the inputs of the UIC. */
return cpu;
}
--
2.20.1
Re: [PATCH v2] hw/core: Restrict 'fw-path-provider.c' to system mode emulation
On 07/12/20 23:07, Philippe Mathieu-Daudé wrote:
fw-path-provider.c is only consumed by qdev-fw.c, which itself
is in softmmu_ss[], so we can restrict fw-path-provider.c to
softmmu too.
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Fix author email.
---
hw/core/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/core/meson.build b/hw/core/meson.build
index 4a744f3b5e7..032576f5717 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -1,7 +1,6 @@
# core qdev-related obj files, also used by *-user and unit tests
hwcore_files = files(
'bus.c',
- 'fw-path-provider.c',
'hotplug.c',
'qdev-properties.c',
'qdev.c',
@@ -25,6 +24,7 @@
common_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
softmmu_ss.add(files(
+ 'fw-path-provider.c',
'loader.c',
'machine-hmp-cmds.c',
'machine.c',
Queued, thanks.
paolo
Re: checkpatch.pl block comment detection fail
On Fri, 11 Dec 2020 at 22:54, Doug Evans wrote: > > Hi. > > The coding style docs don't specify this as being bad: > > foo(/*bar=*/true); > > which improves readability at the call site. > It's not a style to be used liberally, but sometimes it's helpful. > > Alas checkpatch.pl claims this is a block comment. > > Would it be ok if I try to fix checkpatch.pl to treat this as ok? Well, whether it's good or bad style it's certainly not a block comment, so it's a bug for checkpatch to call it one. If you want to dive into that morass of perl feel free :-) thanks -- PMM
Re: [PATCH 14/15] null-machine: do not create a default memdev
On 07/12/20 17:43, Igor Mammedov wrote:
mc->default_ram_size = 0;
-mc->default_ram_id = "ram";
+mc->default_ram_id = NULL;
probably that will break:
QEMU -m X -M none
No, it works. "-m" is simply ignored, because the default memdev is
created here:
if (machine_class->default_ram_id && current_machine->ram_size &&
numa_uses_legacy_mem() && !current_machine->ram_memdev_id) {
create_default_memdev(current_machine, mem_path);
}
and is thus skipped for -M none.
Paolo
maybe there is a bug over there but
"mc->default_ram_size = 0"
above, should result in
current_machine->ram_size == 0
in case user hasn't provided "-m"
and hence memdev shouldn't be created
Re: [PATCH 3/3] sev: update sev-inject-launch-secret to make gpa optional
On 12/11/20 4:45 PM, James Bottomley wrote: On Fri, 2020-12-11 at 16:00 -0600, Tom Lendacky wrote: On 12/9/20 11:23 AM, James Bottomley wrote: So for this one I'm not checking the length, which argues it wouldn't be subject to the added length new data rule and I'd have to use a new guid for new information. However, I could also see situations where you would check the length and thus would have the ability to add fields (either at the beginning or the end). I think this paragraph explains it nicely and a slightly expanded comment with this information would be enough. Thanks, Tom Whatever we decide should probably be documented in both the OVMF patches and the Qemu patches. OK, I can add a comment about my use case and you can add one documenting your length based use case. James
Re: [PATCH 3/3] sev: update sev-inject-launch-secret to make gpa optional
On Fri, 2020-12-11 at 16:00 -0600, Tom Lendacky wrote:
> On 12/9/20 11:23 AM, James Bottomley wrote:
> > If the gpa isn't specified, it's value is extracted from the OVMF
> > properties table located below the reset vector (and if this
> > doesn't
> > exist, an error is returned). OVMF has defined the GUID for the
> > SEV
> > secret area as 4c2eb361-7d9b-4cc3-8081-127c90d3d294 and the format
> > of
> > the is: | where both are uint32_t. We extract
> > and use it as the gpa for the injection.
> >
> > Note: it is expected that the injected secret will also be GUID
> > described but since qemu can't interpret it, the format is left
> > undefined here.
> >
> > Signed-off-by: James Bottomley
> > ---
> > qapi/misc-target.json | 2 +-
> > target/i386/monitor.c | 22 +-
> > 2 files changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index 4486a543ae..1ee4e62f85 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -216,7 +216,7 @@
> > #
> > ##
> > { 'command': 'sev-inject-launch-secret',
> > - 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa':
> > 'uint64' },
> > + 'data': { 'packet-header': 'str', 'secret': 'str', '*gpa':
> > 'uint64' },
> > 'if': 'defined(TARGET_I386)' }
> >
> > ##
> > diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> > index 1bc91442b1..a99e3dd2b3 100644
> > --- a/target/i386/monitor.c
> > +++ b/target/i386/monitor.c
> > @@ -34,6 +34,7 @@
> > #include "sev_i386.h"
> > #include "qapi/qapi-commands-misc-target.h"
> > #include "qapi/qapi-commands-misc.h"
> > +#include "hw/i386/pc.h"
> >
> > /* Perform linear address sign extension */
> > static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
> > @@ -730,9 +731,28 @@ SevCapability
> > *qmp_query_sev_capabilities(Error **errp)
> > return sev_get_capabilities(errp);
> > }
> >
> > +#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
> > +struct sev_secret_area {
> > +uint32_t base;
> > +uint32_t size;
> > +};
> > +
>
> Originally, the idea was to allow expanding of these GUID based
> structures by pre-pending data to them, but based on how
> pc_system_ovmf_table_find() returns the pointer to the start of the
> structure (based on the length found in the structure), I believe
> that expansion could be done by appending to the structure, which
> seems more logical. For example, if this structure is ever expanded,
> it can use the third parameter of pc_system_ovmf_table_find() to get
> the length and compare that to the size of the structure to
> determine if new version of the structure is present in the firmware.
Actually, I don't think it much matters. It looks like the len it
would return is wrong ... it should be the length of just the returned
data pointer (without the length or guid), so ptr+len would point to
the foot of the data if that's what you want.
> Otherwise you can't do the nice easy assignment below:
>area = (struct sev_secret_area *)data;
>
> You actually have to do some math:
>area = (struct sev_secret_area *)(data + data_len -
> sizeof(QemuUUID) -
> sizeof(uint16_t) -
> sizeof(*area));
>
> or add the QemuUUID and uint16_t fields to sev_secret_area and:
>area = (struct sev_secret_area *)(data + data_len -
> sizeof(*area));
Right, that's why I think patch 2/3 should do
*data_len = len - sizeof(QemuUUID) - sizeof(uint16_t)
> Or we make the decision that these GUID structs should never change,
> just add a new one to the table if more info is needed.
Actually, the fact that the only guid the table depends on is the table
footer GUID, you can always remove guids and add new ones. So I think
it's up to whoever is using the GUID to decide the policy.
So for this one I'm not checking the length, which argues it wouldn't
be subject to the added length new data rule and I'd have to use a new
guid for new information. However, I could also see situations where
you would check the length and thus would have the ability to add
fields (either at the beginning or the end).
> Whatever we decide should probably be documented in both the OVMF
> patches and the Qemu patches.
OK, I can add a comment about my use case and you can add one
documenting your length based use case.
James
Re: [PATCH 3/3] sev: update sev-inject-launch-secret to make gpa optional
On 12/9/20 11:23 AM, James Bottomley wrote:
If the gpa isn't specified, it's value is extracted from the OVMF
properties table located below the reset vector (and if this doesn't
exist, an error is returned). OVMF has defined the GUID for the SEV
secret area as 4c2eb361-7d9b-4cc3-8081-127c90d3d294 and the format of
the is: | where both are uint32_t. We extract
and use it as the gpa for the injection.
Note: it is expected that the injected secret will also be GUID
described but since qemu can't interpret it, the format is left
undefined here.
Signed-off-by: James Bottomley
---
qapi/misc-target.json | 2 +-
target/i386/monitor.c | 22 +-
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 4486a543ae..1ee4e62f85 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -216,7 +216,7 @@
#
##
{ 'command': 'sev-inject-launch-secret',
- 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
+ 'data': { 'packet-header': 'str', 'secret': 'str', '*gpa': 'uint64' },
'if': 'defined(TARGET_I386)' }
##
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 1bc91442b1..a99e3dd2b3 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -34,6 +34,7 @@
#include "sev_i386.h"
#include "qapi/qapi-commands-misc-target.h"
#include "qapi/qapi-commands-misc.h"
+#include "hw/i386/pc.h"
/* Perform linear address sign extension */
static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
@@ -730,9 +731,28 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
return sev_get_capabilities(errp);
}
+#define SEV_SECRET_GUID "4c2eb361-7d9b-4cc3-8081-127c90d3d294"
+struct sev_secret_area {
+uint32_t base;
+uint32_t size;
+};
+
Originally, the idea was to allow expanding of these GUID based structures
by pre-pending data to them, but based on how pc_system_ovmf_table_find()
returns the pointer to the start of the structure (based on the length
found in the structure), I believe that expansion could be done by
appending to the structure, which seems more logical. For example, if this
structure is ever expanded, it can use the third parameter of
pc_system_ovmf_table_find() to get the length and compare that to the size
of the structure to determine if new version of the structure is present
in the firmware.
Otherwise you can't do the nice easy assignment below:
area = (struct sev_secret_area *)data;
You actually have to do some math:
area = (struct sev_secret_area *)(data + data_len -
sizeof(QemuUUID) - sizeof(uint16_t) -
sizeof(*area));
or add the QemuUUID and uint16_t fields to sev_secret_area and:
area = (struct sev_secret_area *)(data + data_len - sizeof(*area));
Or we make the decision that these GUID structs should never change, just
add a new one to the table if more info is needed.
Whatever we decide should probably be documented in both the OVMF patches
and the Qemu patches.
Thanks,
Tom
void qmp_sev_inject_launch_secret(const char *packet_hdr,
- const char *secret, uint64_t gpa,
+ const char *secret,
+ bool has_gpa, uint64_t gpa,
Error **errp)
{
+if (!has_gpa) {
+uint8_t *data;
+struct sev_secret_area *area;
+
+if (!pc_system_ovmf_table_find(SEV_SECRET_GUID, &data, NULL)) {
+error_setg(errp, "SEV: no secret area found in OVMF, gpa must be
specified.");
+return;
+}
+area = (struct sev_secret_area *)data;
+gpa = area->base;
+}
+
sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
}
Re: [PATCH v3 0/4] Add a new -action parameter
On 11/12/20 17:52, Alejandro Jimenez wrote: This is a follow up to the proposal to add a "-no-panicstop" option to QEMU that would allow us to control whether the VM is paused or allowed to continue running without intervention from a management layer when a guest panic occurs. See the inital thread and replies for details: https://lore.kernel.org/qemu-devel/1601606494-1154-1-git-send-email-alejandro.j.jime...@oracle.com/ From that discussion came a request for a generic mechanism to group options like -no-shutdown, -no-reboot, etc, that specify an action taken by QEMU in response to a guest event (reboot, shutdown, panic, and watchdog expiration are the current options). The existing options would translate to the new option, like: * -no-reboot --> "-action reboot=shutdown" * -no-shutdown --> "-action shutdown=pause" Please share any questions or comments. Thanks, this looks good. (Actually there are a bunch of changes needed for other patches that I have queued, but I can take care of that. Basically, I am moving command line parsing from softmmu/runstate-action.c to softmmu/vl.c because we're trying to move all command line stuff there + in turn make vl.c use QMP commands as much as possible). Paolo
Re: [PATCH v3 00/53] Make qdev static property API usable by any QOM type
Paolo, do you have any objections to adding the field property API in this series to QOM, considering the recent discussions about long term plans for QOM properties? Note that I still want to perform most of the changes in this series, even if the code is kept inside `hw/core`. Moving the code to `qom` is just one extra step, but not essential. On Thu, Nov 12, 2020 at 04:42:57PM -0500, Eduardo Habkost wrote: > Based-on: 2020183823.283752-1-ehabk...@redhat.com > Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/qdev-make-generic > > This series refactor the qdev property code so the static > property system from qdev becomes a common QOM API that can be > used by any QOM type. > > As an example, at the end of the series we use the new API at: > * check-qom-proplist unit test, to replace the hand-written > getter and setter for the "sv" property > * target/i386/sev.c, to replace the instance properties > registered using object_property_add_uint32_ptr() > > I still want to make object_class_add_field_properties() the > recommended interface for registering QOM properties, > but this will be done in another series. > > Changes v2 -> v3 > > > * Patches were reordered > > * New qom/qom.h header file > (See "qom: Add new qom.h header") > > * Fixed memory leak in the array property refactor from v2 > (See "qdev: Get rid of ArrayElementProperty struct") > > * Made object_property_add_field() copy the Property struct > passed as argument, to make the array property memory leak > easier to fix > (See "qdev: Make object_property_add_field() copy the Property struct") > > * Both object_class_add_field_properties() and > object_class_property_add_field() functions are available, > but only object_class_property_add_field() are made public > by this series. object_class_add_field_properties() is > used to implement device_class_set_props(). > (See "qom: object_class_property_add_field() function" & > "qdev: Separate generic and device-specific property registration") > > * Now object_class_property_add_field() will get a copy of the > Property struct, to avoid tricks involving static variables > in the FIELD_PROP macro. > (See "qom: object_class_property_add_field() function") > > * check-qom-proplist won't use object_property_add_field(), > as it is an internal API and I don't want to make it a bad > example to be followed. > (See "tests: Use field property at check-qom-proplist test case") > > * Property.qdev_prop_name is now Property.name_template, because > it might be used outside qdev in case we make > object_class_add_field_properties() public. > (See "qdev: Rename Property.name to Property.name_template") > > * New unit test for array property > (See "tests: Add unit test for qdev array properties") > > * Include sev patch for using class properties instead of > object_property_add_uint32_ptr() > (See "sev: Use class properties") > > * FIELD_PROP macro is now inside field-property.h > (See "qom: FIELD_PROP macro") > > * DEFINE_PROP_* macros are now defined using PROP_*, not > the other way around > (See "qom: PROP_* macros") > > Changes v1 -> v2 > > > * Rename functions and source files to call the new feature > "field property" instead of "static property" > > * Change the API signature from an array-based interface similar > to device_class_set_props() to a object_property_add()-like > interface. > > This means instead of doing this: > > object_class_property_add_static_props(oc, my_array_of_props); > > properties are registered like this: > > object_class_property_add_field(oc, "property-name" > PROP_XXX(MyState, my_field), > prop_allow_set_always); > > where PROP_XXX is a PROP_* macro like PROP_STRING, PROP_BOOL, > etc. > > * Make Property.name optional. Rename it to qdev_prop_name, > and restrict its usage to qdev property tracking code. > > * Make allow_set callback mandatory, to avoid ambiguity > > * Big cleanup of array property code. We don't need a custom > release function for array elements anymore, because we don't > need to save the property name in the Property struct anymore. > > * Moved UUID property to qdev-properties-system, because > it still has dependencies on qdev code > > Eduardo Habkost (53): > cs4231: Get rid of empty property array > cpu: Move cpu_common_props to hw/core/cpu.c > qdev: Move property code to qdev-properties.[ch] > qdev: Check dev->realized at set_size() > sparc: Check dev->realized at sparc_set_nwindows() > qdev: Don't use dev->id on set_size32() error message > qdev: Make PropertyInfo.print method get Object* argument > qdev: Make bit_prop_set() get Object* argument > qdev: Make qdev_get_prop_ptr() get Object* arg > qdev: Make qdev_find_global_prop() get Object* argument > qdev: Make check_prop_still_unset() g
[PATCH v2 3/4] hw/adc: Add an ADC module for NPCM7XX
The ADC is part of NPCM7XX Module. Its behavior is controled by the
ADC_CON register. It converts one of the eight analog inputs into a
digital input and stores it in the ADC_DATA register when enabled.
Reviewed-by: Havard Skinnemoen
Reviewed-by: Tyrone Ting
Signed-off-by: Hao Wu
---
docs/system/arm/nuvoton.rst| 2 +-
hw/adc/meson.build | 1 +
hw/adc/npcm7xx_adc.c | 318 ++
hw/arm/npcm7xx.c | 24 +-
include/hw/adc/npcm7xx_adc.h | 72 ++
include/hw/arm/npcm7xx.h | 2 +
tests/qtest/meson.build| 3 +-
tests/qtest/npcm7xx_adc-test.c | 400 +
8 files changed, 819 insertions(+), 3 deletions(-)
create mode 100644 hw/adc/npcm7xx_adc.c
create mode 100644 include/hw/adc/npcm7xx_adc.h
create mode 100644 tests/qtest/npcm7xx_adc-test.c
diff --git a/docs/system/arm/nuvoton.rst b/docs/system/arm/nuvoton.rst
index b00d405d52..35829f8d0b 100644
--- a/docs/system/arm/nuvoton.rst
+++ b/docs/system/arm/nuvoton.rst
@@ -41,6 +41,7 @@ Supported devices
* Random Number Generator (RNG)
* USB host (USBH)
* GPIO controller
+ * Analog to Digital Converter (ADC)
Missing devices
---
@@ -58,7 +59,6 @@ Missing devices
* USB device (USBD)
* SMBus controller (SMBF)
* Peripheral SPI controller (PSPI)
- * Analog to Digital Converter (ADC)
* SD/MMC host
* PECI interface
* Pulse Width Modulation (PWM)
diff --git a/hw/adc/meson.build b/hw/adc/meson.build
index 0d62ae96ae..6ddee23813 100644
--- a/hw/adc/meson.build
+++ b/hw/adc/meson.build
@@ -1 +1,2 @@
softmmu_ss.add(when: 'CONFIG_STM32F2XX_ADC', if_true: files('stm32f2xx_adc.c'))
+softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_adc.c'))
diff --git a/hw/adc/npcm7xx_adc.c b/hw/adc/npcm7xx_adc.c
new file mode 100644
index 00..4492303977
--- /dev/null
+++ b/hw/adc/npcm7xx_adc.c
@@ -0,0 +1,318 @@
+/*
+ * Nuvoton NPCM7xx ADC Module
+ *
+ * Copyright 2020 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "hw/adc/npcm7xx_adc.h"
+#include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/timer.h"
+#include "qemu/units.h"
+
+/* 32-bit register indices. */
+enum NPCM7xxADCRegisters {
+NPCM7XX_ADC_CON,
+NPCM7XX_ADC_DATA,
+NPCM7XX_ADC_REGS_END,
+};
+
+/* Register field definitions. */
+#define NPCM7XX_ADC_CON_MUX(rv) extract32(rv, 24, 4)
+#define NPCM7XX_ADC_CON_INT_EN BIT(21)
+#define NPCM7XX_ADC_CON_REFSEL BIT(19)
+#define NPCM7XX_ADC_CON_INT BIT(18)
+#define NPCM7XX_ADC_CON_EN BIT(17)
+#define NPCM7XX_ADC_CON_RST BIT(16)
+#define NPCM7XX_ADC_CON_CONVBIT(14)
+#define NPCM7XX_ADC_CON_DIV(rv) extract32(rv, 1, 8)
+
+#define NPCM7XX_ADC_MAX_RESULT 1023
+#define NPCM7XX_ADC_DEFAULT_IREF200
+#define NPCM7XX_ADC_CONV_CYCLES 20
+#define NPCM7XX_ADC_RESET_CYCLES10
+#define NPCM7XX_ADC_R0_INPUT50
+#define NPCM7XX_ADC_R1_INPUT150
+
+static void npcm7xx_adc_reset(NPCM7xxADCState *s)
+{
+timer_del(&s->conv_timer);
+timer_del(&s->reset_timer);
+s->con = 0x000c0001;
+s->data = 0x;
+}
+
+static uint32_t npcm7xx_adc_convert(uint32_t input, uint32_t ref)
+{
+uint32_t result;
+
+result = input * (NPCM7XX_ADC_MAX_RESULT + 1) / ref;
+if (result > NPCM7XX_ADC_MAX_RESULT) {
+result = NPCM7XX_ADC_MAX_RESULT;
+}
+
+return result;
+}
+
+static uint32_t npcm7xx_adc_prescaler(NPCM7xxADCState *s)
+{
+return 2 * (NPCM7XX_ADC_CON_DIV(s->con) + 1);
+}
+
+static void npcm7xx_adc_start_timer(Clock *clk, QEMUTimer *timer,
+uint32_t cycles, uint32_t prescaler)
+{
+int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+int64_t freq = clock_get_hz(clk);
+int64_t ns;
+
+ns = (NANOSECONDS_PER_SECOND * cycles * prescaler / freq);
+ns += now;
+timer_mod(timer, ns);
+}
+
+static void npcm7xx_adc_start_reset(NPCM7xxADCState *s)
+{
+uint32_t prescaler = npcm7xx_adc_prescaler(s);
+
+npcm7xx_adc_start_timer(s->clock, &s->reset_timer,
NPCM7XX_ADC_RESET_CYCLES,
+prescaler);
+}
+
+static void npcm7xx_adc_start_convert(NPCM7xxADCState *s)
+{
+uint32_t prescaler = npcm7xx_adc_prescaler(s);
+
+npcm7xx_adc_start_timer(s->clock, &s->conv_timer, NPCM7XX_ADC_CONV_CYCLES,
+prescaler);
+}
+
+static void npcm7xx_adc_reset_done(void *opaque)
+{
+NPCM7xxADCState *s = opaque;
+
+npcm7xx_adc_
Re: [PATCH v3 00/53] Make qdev static property API usable by any QOM type
On 11/12/20 23:08, Eduardo Habkost wrote: Paolo, do you have any objections to adding the field property API in this series to QOM, considering the recent discussions about long term plans for QOM properties? Note that I still want to perform most of the changes in this series, even if the code is kept inside `hw/core`. Moving the code to `qom` is just one extra step, but not essential. Absolutely not, go ahead. Paolo
[PATCH v2 1/4] hw/misc: Add clock converter in NPCM7XX CLK module
This patch allows NPCM7XX CLK module to compute clocks that are used by
other NPCM7XX modules.
Add a new struct NPCM7xxClockConverterState which represents a
single converter. Each clock converter in CLK module represents one
converter in NPCM7XX CLK Module(PLL, SEL or Divider). Each converter
takes one or more input clocks and converts them into one output clock.
They form a clock hierarchy in the CLK module and are responsible for
outputing clocks for various other modules in an NPCM7XX SoC.
Each converter has a function pointer called "convert" which represents
the unique logic for that converter.
The clock contains two initialization information: ConverterInitInfo and
ConverterConnectionInfo. They represent the vertices and edges in the
clock diagram respectively.
Reviewed-by: Havard Skinnemoen
Reviewed-by: Tyrone Ting
Signed-off-by: Hao Wu
---
hw/misc/npcm7xx_clk.c | 795 +-
include/hw/misc/npcm7xx_clk.h | 140 +-
2 files changed, 927 insertions(+), 8 deletions(-)
diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
index 6732437fe2..48bc9bdda5 100644
--- a/hw/misc/npcm7xx_clk.c
+++ b/hw/misc/npcm7xx_clk.c
@@ -18,6 +18,7 @@
#include "hw/misc/npcm7xx_clk.h"
#include "hw/timer/npcm7xx_timer.h"
+#include "hw/qdev-clock.h"
#include "migration/vmstate.h"
#include "qemu/error-report.h"
#include "qemu/log.h"
@@ -27,9 +28,22 @@
#include "trace.h"
#include "sysemu/watchdog.h"
+/*
+ * The reference clock hz, and the SECCNT and CNTR25M registers in this module,
+ * is always 25 MHz.
+ */
+#define NPCM7XX_CLOCK_REF_HZ(2500)
+
+/* Register Field Definitions */
+#define NPCM7XX_CLK_WDRCR_CA9C BIT(0) /* Cortex A9 Cores */
+
#define PLLCON_LOKI BIT(31)
#define PLLCON_LOKS BIT(30)
#define PLLCON_PWDENBIT(12)
+#define PLLCON_FBDV(con) extract32((con), 16, 12)
+#define PLLCON_OTDV2(con) extract32((con), 13, 3)
+#define PLLCON_OTDV1(con) extract32((con), 8, 3)
+#define PLLCON_INDV(con) extract32((con), 0, 6)
enum NPCM7xxCLKRegisters {
NPCM7XX_CLK_CLKEN1,
@@ -89,12 +103,609 @@ static const uint32_t
cold_reset_values[NPCM7XX_CLK_NR_REGS] = {
[NPCM7XX_CLK_AHBCKFI] = 0x00c8,
};
-/* Register Field Definitions */
-#define NPCM7XX_CLK_WDRCR_CA9C BIT(0) /* Cortex A9 Cores */
-
/* The number of watchdogs that can trigger a reset. */
#define NPCM7XX_NR_WATCHDOGS(3)
+/* Clock converter functions */
+
+#define TYPE_NPCM7XX_CLOCK_PLL "npcm7xx-clock-pll"
+#define NPCM7XX_CLOCK_PLL(obj) OBJECT_CHECK(NPCM7xxClockPLLState, \
+(obj), TYPE_NPCM7XX_CLOCK_PLL)
+#define TYPE_NPCM7XX_CLOCK_SEL "npcm7xx-clock-sel"
+#define NPCM7XX_CLOCK_SEL(obj) OBJECT_CHECK(NPCM7xxClockSELState, \
+(obj), TYPE_NPCM7XX_CLOCK_SEL)
+#define TYPE_NPCM7XX_CLOCK_DIVIDER "npcm7xx-clock-divider"
+#define NPCM7XX_CLOCK_DIVIDER(obj) OBJECT_CHECK(NPCM7xxClockDividerState, \
+(obj), TYPE_NPCM7XX_CLOCK_DIVIDER)
+
+static void npcm7xx_clk_update_pll(void *opaque)
+{
+NPCM7xxClockPLLState *s = opaque;
+uint32_t con = s->clk->regs[s->reg];
+uint64_t freq;
+
+/* The PLL is grounded if it is not locked yet. */
+if (con & PLLCON_LOKI) {
+freq = clock_get_hz(s->clock_in);
+freq *= PLLCON_FBDV(con);
+freq /= PLLCON_INDV(con) * PLLCON_OTDV1(con) * PLLCON_OTDV2(con);
+} else {
+freq = 0;
+}
+
+clock_update_hz(s->clock_out, freq);
+}
+
+static void npcm7xx_clk_update_sel(void *opaque)
+{
+NPCM7xxClockSELState *s = opaque;
+uint32_t index = extract32(s->clk->regs[NPCM7XX_CLK_CLKSEL], s->offset,
+s->len);
+
+if (index >= s->input_size) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: SEL index: %u out of range\n",
+ __func__, index);
+index = 0;
+}
+clock_update_hz(s->clock_out, clock_get_hz(s->clock_in[index]));
+}
+
+static void npcm7xx_clk_update_divider(void *opaque)
+{
+NPCM7xxClockDividerState *s = opaque;
+uint32_t freq;
+
+freq = s->divide(s);
+clock_update_hz(s->clock_out, freq);
+}
+
+static uint32_t divide_by_constant(NPCM7xxClockDividerState *s)
+{
+return clock_get_hz(s->clock_in) / s->divisor;
+}
+
+static uint32_t divide_by_reg_divisor(NPCM7xxClockDividerState *s)
+{
+return clock_get_hz(s->clock_in) /
+(extract32(s->clk->regs[s->reg], s->offset, s->len) + 1);
+}
+
+static uint32_t divide_by_reg_divisor_times_2(NPCM7xxClockDividerState *s)
+{
+return divide_by_reg_divisor(s) / 2;
+}
+
+static uint32_t shift_by_reg_divisor(NPCM7xxClockDividerState *s)
+{
+return clock_get_hz(s->clock_in) >>
+extract32(s->clk->regs[s->reg], s->offset, s->len);
+}
+
+static NPCM7xxClockPLL find_pll_by_reg(enum NPCM7xxCLKRegisters reg)
+{
+switch (reg) {
+case NPCM7XX_CLK_PLLCON0:
+return NPCM7XX_CLOCK_PLL0;
+case NPCM7XX_CLK_PLLCON1:
+return NPCM7XX_CLOCK_PLL1;
+case NPCM7XX_CLK_PLLCON2:
+return N
[PATCH v4 32/32] tests: Add unit test for qdev array properties
Add a test case to ensure array properties are behaving as
expected.
Signed-off-by: Eduardo Habkost
---
This is a new patch added in v3 of this series.
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
tests/test-qdev-global-props.c | 61 ++
1 file changed, 61 insertions(+)
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index c8862cac5f..9426ce2a72 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -44,11 +44,16 @@ struct MyType {
uint32_t prop1;
uint32_t prop2;
+
+char **myarray;
+uint32_t myarray_len;
};
static Property static_props[] = {
DEFINE_PROP_UINT32("prop1", MyType, prop1, PROP_DEFAULT),
DEFINE_PROP_UINT32("prop2", MyType, prop2, PROP_DEFAULT),
+DEFINE_PROP_ARRAY("myarray", MyType, myarray_len, myarray,
+ prop_info_string, char *),
DEFINE_PROP_END_OF_LIST()
};
@@ -60,11 +65,19 @@ static void static_prop_class_init(ObjectClass *oc, void
*data)
device_class_set_props(dc, static_props);
}
+static void static_props_finalize(Object *obj)
+{
+MyType *mt = STATIC_TYPE(obj);
+
+g_free(mt->myarray);
+}
+
static const TypeInfo static_prop_type = {
.name = TYPE_STATIC_PROPS,
.parent = TYPE_DEVICE,
.instance_size = sizeof(MyType),
.class_init = static_prop_class_init,
+.instance_finalize = static_props_finalize,
};
static const TypeInfo subclass_type = {
@@ -91,6 +104,52 @@ static void test_static_prop(void)
g_test_trap_assert_stdout("");
}
+static void test_static_prop_array(void)
+{
+Error *err = NULL;
+ObjectClass *oc = object_class_by_name(TYPE_STATIC_PROPS);
+Object *obj = object_new(TYPE_STATIC_PROPS);
+char *s = NULL;
+
+g_assert_nonnull(object_class_property_find(oc, "len-myarray"));
+g_assert_null(object_class_property_find(oc, "myarray[0]"));
+
+g_assert_nonnull(object_property_find(obj, "len-myarray"));
+g_assert_null(object_property_find(obj, "myarray[0]"));
+
+g_assert_cmpint(object_property_get_int(obj, "len-myarray", &error_abort),
==, 0);
+object_property_set_int(obj, "len-myarray", 3, &error_abort);
+g_assert_cmpint(object_property_get_int(obj, "len-myarray", &error_abort),
==, 3);
+
+g_assert_nonnull(object_property_find(obj, "myarray[0]"));
+g_assert_nonnull(object_property_find(obj, "myarray[1]"));
+g_assert_nonnull(object_property_find(obj, "myarray[2]"));
+g_assert_null(object_property_find(obj, "myarray[3]"));
+
+/* Setting length a second time must fail */
+object_property_set_int(obj, "len-myarray", 42, &err);
+error_free_or_abort(&err);
+
+g_assert_nonnull(object_property_find(obj, "myarray[2]"));
+g_assert_null(object_property_find(obj, "myarray[3]"));
+
+s = object_property_get_str(obj, "myarray[2]", &error_abort);
+g_assert_cmpstr(s, ==, "");
+g_free(s);
+
+object_property_set_str(obj, "myarray[1]", "value", &error_abort);
+
+s = object_property_get_str(obj, "myarray[1]", &error_abort);
+g_assert_cmpstr(s, ==, "value");
+g_free(s);
+
+s = object_property_get_str(obj, "myarray[2]", &error_abort);
+g_assert_cmpstr(s, ==, "");
+g_free(s);
+
+object_unref(obj);
+}
+
static void register_global_properties(GlobalProperty *props)
{
int i;
@@ -299,6 +358,8 @@ int main(int argc, char **argv)
test_static_prop_subprocess);
g_test_add_func("/qdev/properties/static/default",
test_static_prop);
+g_test_add_func("/qdev/properties/static/array",
+test_static_prop_array);
g_test_add_func("/qdev/properties/static/global/subprocess",
test_static_globalprop_subprocess);
--
2.28.0
[PATCH v2 2/4] hw/timer: Refactor NPCM7XX Timer to use CLK clock
This patch makes NPCM7XX Timer to use a the timer clock generated by the
CLK module instead of the magic nubmer TIMER_REF_HZ.
Reviewed-by: Havard Skinnemoen
Reviewed-by: Tyrone Ting
Signed-off-by: Hao Wu
---
hw/arm/npcm7xx.c | 5 +
hw/timer/npcm7xx_timer.c | 25 +++--
include/hw/misc/npcm7xx_clk.h| 6 --
include/hw/timer/npcm7xx_timer.h | 1 +
4 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 47e2b6fc40..fabfb1697b 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -22,6 +22,7 @@
#include "hw/char/serial.h"
#include "hw/loader.h"
#include "hw/misc/unimp.h"
+#include "hw/qdev-clock.h"
#include "hw/qdev-properties.h"
#include "qapi/error.h"
#include "qemu/units.h"
@@ -420,6 +421,10 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
int first_irq;
int j;
+/* Connect the timer clock. */
+qdev_connect_clock_in(DEVICE(&s->tim[i]), "clock", qdev_get_clock_out(
+DEVICE(&s->clk), "timer-clock"));
+
sysbus_realize(sbd, &error_abort);
sysbus_mmio_map(sbd, 0, npcm7xx_tim_addr[i]);
diff --git a/hw/timer/npcm7xx_timer.c b/hw/timer/npcm7xx_timer.c
index d24445bd6e..9469c959e2 100644
--- a/hw/timer/npcm7xx_timer.c
+++ b/hw/timer/npcm7xx_timer.c
@@ -17,8 +17,8 @@
#include "qemu/osdep.h"
#include "hw/irq.h"
+#include "hw/qdev-clock.h"
#include "hw/qdev-properties.h"
-#include "hw/misc/npcm7xx_clk.h"
#include "hw/timer/npcm7xx_timer.h"
#include "migration/vmstate.h"
#include "qemu/bitops.h"
@@ -130,7 +130,7 @@ static int64_t npcm7xx_timer_count_to_ns(NPCM7xxTimer *t,
uint32_t count)
{
int64_t ns = count;
-ns *= NANOSECONDS_PER_SECOND / NPCM7XX_TIMER_REF_HZ;
+ns *= NANOSECONDS_PER_SECOND / clock_get_hz(t->ctrl->clock);
ns *= npcm7xx_tcsr_prescaler(t->tcsr);
return ns;
@@ -141,7 +141,7 @@ static uint32_t npcm7xx_timer_ns_to_count(NPCM7xxTimer *t,
int64_t ns)
{
int64_t count;
-count = ns / (NANOSECONDS_PER_SECOND / NPCM7XX_TIMER_REF_HZ);
+count = ns / (NANOSECONDS_PER_SECOND / clock_get_hz(t->ctrl->clock));
count /= npcm7xx_tcsr_prescaler(t->tcsr);
return count;
@@ -166,8 +166,10 @@ static uint32_t npcm7xx_watchdog_timer_prescaler(const
NPCM7xxWatchdogTimer *t)
static void npcm7xx_watchdog_timer_reset_cycles(NPCM7xxWatchdogTimer *t,
int64_t cycles)
{
+g_assert(clock_get_hz(t->ctrl->clock) == 2500);
uint32_t prescaler = npcm7xx_watchdog_timer_prescaler(t);
-int64_t ns = (NANOSECONDS_PER_SECOND / NPCM7XX_TIMER_REF_HZ) * cycles;
+int64_t ns = (NANOSECONDS_PER_SECOND / clock_get_hz(t->ctrl->clock))
+* cycles;
/*
* The reset function always clears the current timer. The caller of the
@@ -606,10 +608,11 @@ static void npcm7xx_timer_hold_reset(Object *obj)
qemu_irq_lower(s->watchdog_timer.irq);
}
-static void npcm7xx_timer_realize(DeviceState *dev, Error **errp)
+static void npcm7xx_timer_init(Object *obj)
{
-NPCM7xxTimerCtrlState *s = NPCM7XX_TIMER(dev);
+NPCM7xxTimerCtrlState *s = NPCM7XX_TIMER(obj);
SysBusDevice *sbd = &s->parent;
+DeviceState *dev = DEVICE(obj);
int i;
NPCM7xxWatchdogTimer *w;
@@ -627,11 +630,12 @@ static void npcm7xx_timer_realize(DeviceState *dev, Error
**errp)
npcm7xx_watchdog_timer_expired, w);
sysbus_init_irq(sbd, &w->irq);
-memory_region_init_io(&s->iomem, OBJECT(s), &npcm7xx_timer_ops, s,
+memory_region_init_io(&s->iomem, obj, &npcm7xx_timer_ops, s,
TYPE_NPCM7XX_TIMER, 4 * KiB);
sysbus_init_mmio(sbd, &s->iomem);
qdev_init_gpio_out_named(dev, &w->reset_signal,
NPCM7XX_WATCHDOG_RESET_GPIO_OUT, 1);
+s->clock = qdev_init_clock_in(dev, "clock", NULL, NULL);
}
static const VMStateDescription vmstate_npcm7xx_base_timer = {
@@ -675,10 +679,11 @@ static const VMStateDescription
vmstate_npcm7xx_watchdog_timer = {
static const VMStateDescription vmstate_npcm7xx_timer_ctrl = {
.name = "npcm7xx-timer-ctrl",
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
.fields = (VMStateField[]) {
VMSTATE_UINT32(tisr, NPCM7xxTimerCtrlState),
+VMSTATE_CLOCK(clock, NPCM7xxTimerCtrlState),
VMSTATE_STRUCT_ARRAY(timer, NPCM7xxTimerCtrlState,
NPCM7XX_TIMERS_PER_CTRL, 0, vmstate_npcm7xx_timer,
NPCM7xxTimer),
@@ -697,7 +702,6 @@ static void npcm7xx_timer_class_init(ObjectClass *klass,
void *data)
QEMU_BUILD_BUG_ON(NPCM7XX_TIMER_REGS_END > NPCM7XX_TIMER_NR_REGS);
dc->desc = "NPCM7xx Timer Controller";
-dc->realize = npcm7xx_timer_realize;
dc->vmsd = &vmstate_npcm7xx_timer_ctrl;
rc->phases.enter = npcm7xx_timer_enter_reset;
rc->phases.hold = npcm7xx_timer_hold_reset;
@@ -708,6 +712,7 @@ static const Type
[PATCH v2 4/4] hw/misc: Add a PWM module for NPCM7XX
The PWM module is part of NPCM7XX module. Each NPCM7XX module has two
identical PWM modules. Each module contains 4 PWM entries. Each PWM has
two outputs: frequency and duty_cycle. Both are computed using inputs
from software side.
This module does not model detail pulse signals since it is expensive.
It also does not model interrupts and watchdogs that are dependant on
the detail models. The interfaces for these are left in the module so
that anyone in need for these functionalities can implement on their
own.
Reviewed-by: Havard Skinnemoen
Reviewed-by: Tyrone Ting
Signed-off-by: Hao Wu
---
docs/system/arm/nuvoton.rst| 2 +-
hw/arm/npcm7xx.c | 26 +-
hw/misc/meson.build| 1 +
hw/misc/npcm7xx_pwm.c | 535 +
include/hw/arm/npcm7xx.h | 2 +
include/hw/misc/npcm7xx_pwm.h | 106 +++
tests/qtest/meson.build| 1 +
tests/qtest/npcm7xx_pwm-test.c | 490 ++
8 files changed, 1160 insertions(+), 3 deletions(-)
create mode 100644 hw/misc/npcm7xx_pwm.c
create mode 100644 include/hw/misc/npcm7xx_pwm.h
create mode 100644 tests/qtest/npcm7xx_pwm-test.c
diff --git a/docs/system/arm/nuvoton.rst b/docs/system/arm/nuvoton.rst
index 35829f8d0b..a1786342e2 100644
--- a/docs/system/arm/nuvoton.rst
+++ b/docs/system/arm/nuvoton.rst
@@ -42,6 +42,7 @@ Supported devices
* USB host (USBH)
* GPIO controller
* Analog to Digital Converter (ADC)
+ * Pulse Width Modulation (PWM)
Missing devices
---
@@ -61,7 +62,6 @@ Missing devices
* Peripheral SPI controller (PSPI)
* SD/MMC host
* PECI interface
- * Pulse Width Modulation (PWM)
* Tachometer
* PCI and PCIe root complex and bridges
* VDM and MCTP support
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index b22a8c966d..72040d4079 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -102,6 +102,8 @@ enum NPCM7xxInterrupt {
NPCM7XX_WDG2_IRQ, /* Timer Module 2 Watchdog */
NPCM7XX_EHCI_IRQ= 61,
NPCM7XX_OHCI_IRQ= 62,
+NPCM7XX_PWM0_IRQ= 93, /* PWM module 0 */
+NPCM7XX_PWM1_IRQ, /* PWM module 1 */
NPCM7XX_GPIO0_IRQ = 116,
NPCM7XX_GPIO1_IRQ,
NPCM7XX_GPIO2_IRQ,
@@ -144,6 +146,12 @@ static const hwaddr npcm7xx_fiu3_flash_addr[] = {
0xb800, /* CS3 */
};
+/* Register base address for each PWM Module */
+static const hwaddr npcm7xx_pwm_addr[] = {
+0xf0103000,
+0xf0104000,
+};
+
static const struct {
hwaddr regs_addr;
uint32_t unconnected_pins;
@@ -353,6 +361,10 @@ static void npcm7xx_init(Object *obj)
object_initialize_child(obj, npcm7xx_fiu[i].name, &s->fiu[i],
TYPE_NPCM7XX_FIU);
}
+
+for (i = 0; i < ARRAY_SIZE(s->pwm); i++) {
+object_initialize_child(obj, "pwm[*]", &s->pwm[i], TYPE_NPCM7XX_PWM);
+}
}
static void npcm7xx_realize(DeviceState *dev, Error **errp)
@@ -513,6 +525,18 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->ohci), 0,
npcm7xx_irq(s, NPCM7XX_OHCI_IRQ));
+/* PWM Modules. Cannot fail. */
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(npcm7xx_pwm_addr) != ARRAY_SIZE(s->pwm));
+for (i = 0; i < ARRAY_SIZE(s->pwm); i++) {
+SysBusDevice *sbd = SYS_BUS_DEVICE(&s->pwm[i]);
+
+qdev_connect_clock_in(DEVICE(&s->pwm[i]), "clock", qdev_get_clock_out(
+DEVICE(&s->clk), "apb3-clock"));
+sysbus_realize(sbd, &error_abort);
+sysbus_mmio_map(sbd, 0, npcm7xx_pwm_addr[i]);
+sysbus_connect_irq(sbd, i, npcm7xx_irq(s, NPCM7XX_PWM0_IRQ + i));
+}
+
/*
* Flash Interface Unit (FIU). Can fail if incorrect number of chip selects
* specified, but this is a programming error.
@@ -580,8 +604,6 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("npcm7xx.peci", 0xf010, 4 * KiB);
create_unimplemented_device("npcm7xx.siox[1]", 0xf0101000, 4 * KiB);
create_unimplemented_device("npcm7xx.siox[2]", 0xf0102000, 4 * KiB);
-create_unimplemented_device("npcm7xx.pwm[0]", 0xf0103000, 4 * KiB);
-create_unimplemented_device("npcm7xx.pwm[1]", 0xf0104000, 4 * KiB);
create_unimplemented_device("npcm7xx.mft[0]", 0xf018, 4 * KiB);
create_unimplemented_device("npcm7xx.mft[1]", 0xf0181000, 4 * KiB);
create_unimplemented_device("npcm7xx.mft[2]", 0xf0182000, 4 * KiB);
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index ce15ffceb9..607cd38a21 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -64,6 +64,7 @@ softmmu_ss.add(when: 'CONFIG_MAINSTONE', if_true:
files('mst_fpga.c'))
softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files(
'npcm7xx_clk.c',
'npcm7xx_gcr.c',
+ 'npcm7xx_pwm.c',
'npcm7xx_rng.c',
))
softmmu_ss.add(wh
[PATCH v4 30/32] qdev: Rename qdev_get_prop_ptr() to object_field_prop_ptr()
The function will be moved to common QOM code, as it is not
specific to TYPE_DEVICE anymore.
Reviewed-by: Stefan Berger
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Rename to object_field_prop_ptr() instead of object_static_prop_ptr()
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Thomas Huth
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: qemu-devel@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
backends/tpm/tpm_util.c | 6 ++--
hw/block/xen-block.c | 4 +--
hw/core/qdev-properties-system.c | 50 +-
hw/core/qdev-properties.c| 60
hw/s390x/css.c | 4 +--
hw/s390x/s390-pci-bus.c | 4 +--
hw/vfio/pci-quirks.c | 4 +--
8 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 90222822f1..97bb9494ae 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -193,7 +193,7 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name,
const uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-void *qdev_get_prop_ptr(Object *obj, Property *prop);
+void *object_field_prop_ptr(Object *obj, Property *prop);
void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(Object *obj,
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 39b45fa46d..a6e6d3e72f 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -35,7 +35,7 @@
static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-TPMBackend **be = qdev_get_prop_ptr(obj, opaque);
+TPMBackend **be = object_field_prop_ptr(obj, opaque);
char *p;
p = g_strdup(*be ? (*be)->id : "");
@@ -47,7 +47,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
Error **errp)
{
Property *prop = opaque;
-TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
+TPMBackend *s, **be = object_field_prop_ptr(obj, prop);
char *str;
if (!visit_type_str(v, name, &str, errp)) {
@@ -67,7 +67,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void release_tpm(Object *obj, const char *name, void *opaque)
{
Property *prop = opaque;
-TPMBackend **be = qdev_get_prop_ptr(obj, prop);
+TPMBackend **be = object_field_prop_ptr(obj, prop);
if (*be) {
tpm_backend_reset(*be);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index bd1aef63a7..718d886e5c 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -336,7 +336,7 @@ static void xen_block_get_vdev(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
+XenBlockVdev *vdev = object_field_prop_ptr(obj, prop);
char *str;
switch (vdev->type) {
@@ -396,7 +396,7 @@ static void xen_block_set_vdev(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
+XenBlockVdev *vdev = object_field_prop_ptr(obj, prop);
char *str, *p;
const char *end;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 590c5f3d97..e6d378a34e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -62,7 +62,7 @@ static void get_drive(Object *obj, Visitor *v, const char
*name, void *opaque,
Error **errp)
{
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(obj, prop);
+void **ptr = object_field_prop_ptr(obj, prop);
const char *value;
char *p;
@@ -88,7 +88,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(obj, prop);
+void **ptr = object_field_prop_ptr(obj, prop);
char *str;
BlockBackend *blk;
bool blk_created = false;
@@ -181,7 +181,7 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-BlockBackend **ptr = qdev_get_prop_ptr(obj, prop);
+BlockBackend **ptr = object_field_prop_ptr(obj, prop);
if (*ptr) {
AioContext *ctx = blk_get_aio_context(*ptr);
@@ -214,7 +214,
[PATCH v4 31/32] qdev: Avoid unnecessary DeviceState* variable at set_prop_arraylen()
We're just doing pointer math with the device pointer, we can simply use obj instead. Reviewed-by: Marc-André Lureau Signed-off-by: Eduardo Habkost --- Cc: Paolo Bonzini Cc: "Daniel P. Berrangé" Cc: Eduardo Habkost Cc: qemu-devel@nongnu.org --- hw/core/qdev-properties.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 3d648b088d..9d25b49fc1 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -559,10 +559,9 @@ static void set_prop_arraylen(Object *obj, Visitor *v, const char *name, * array-length field in the device struct, we have to create the * array itself and dynamically add the corresponding properties. */ -DeviceState *dev = DEVICE(obj); Property *prop = opaque; uint32_t *alenptr = object_field_prop_ptr(obj, prop); -void **arrayptr = (void *)dev + prop->arrayoffset; +void **arrayptr = (void *)obj + prop->arrayoffset; void *eltptr; const char *arrayname; int i; @@ -602,7 +601,7 @@ static void set_prop_arraylen(Object *obj, Visitor *v, const char *name, * they get the right answer despite the array element not actually * being inside the device struct. */ -arrayprop->prop.offset = eltptr - (void *)dev; +arrayprop->prop.offset = eltptr - (void *)obj; assert(object_field_prop_ptr(obj, &arrayprop->prop) == eltptr); object_property_add(obj, propname, arrayprop->prop.info->name, -- 2.28.0
[PATCH v2 0/4] Additional NPCM7xx devices
This patch series include a few more NPCM7XX devices including - Analog Digital Converter (ADC) - Pulse Width Modulation (PWM) We also modified the CLK module to generate clock values using qdev_clock. These clocks are used to determine various clocks in NPCM7XX devices. Thank you for your review! Changes since v1: We removed the IPMI and KCS related code from this patch set. Hao Wu (4): hw/misc: Add clock converter in NPCM7XX CLK module hw/timer: Refactor NPCM7XX Timer to use CLK clock hw/adc: Add an ADC module for NPCM7XX hw/misc: Add a PWM module for NPCM7XX docs/system/arm/nuvoton.rst | 4 +- hw/adc/meson.build | 1 + hw/adc/npcm7xx_adc.c | 318 + hw/arm/npcm7xx.c | 55 ++- hw/misc/meson.build | 1 + hw/misc/npcm7xx_clk.c| 795 ++- hw/misc/npcm7xx_pwm.c| 535 + hw/timer/npcm7xx_timer.c | 25 +- include/hw/adc/npcm7xx_adc.h | 72 +++ include/hw/arm/npcm7xx.h | 4 + include/hw/misc/npcm7xx_clk.h| 146 +- include/hw/misc/npcm7xx_pwm.h| 106 + include/hw/timer/npcm7xx_timer.h | 1 + tests/qtest/meson.build | 4 +- tests/qtest/npcm7xx_adc-test.c | 400 tests/qtest/npcm7xx_pwm-test.c | 490 +++ 16 files changed, 2927 insertions(+), 30 deletions(-) create mode 100644 hw/adc/npcm7xx_adc.c create mode 100644 hw/misc/npcm7xx_pwm.c create mode 100644 include/hw/adc/npcm7xx_adc.h create mode 100644 include/hw/misc/npcm7xx_pwm.h create mode 100644 tests/qtest/npcm7xx_adc-test.c create mode 100644 tests/qtest/npcm7xx_pwm-test.c -- 2.29.2.684.gfbc64c5ab5-goog
checkpatch.pl block comment detection fail
Hi. The coding style docs don't specify this as being bad: foo(/*bar=*/true); which improves readability at the call site. It's not a style to be used liberally, but sometimes it's helpful. Alas checkpatch.pl claims this is a block comment. Would it be ok if I try to fix checkpatch.pl to treat this as ok?
[PATCH v4 27/32] qdev: Rename qdev_propinfo_* to field_prop_*
These functions will be moved to be part of QOM, so rename them.
Signed-off-by: Eduardo Habkost
---
Changes v2:
* Rename to field_prop_* instead of object_propinfo_*
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-prop-internal.h | 28 +++
hw/core/qdev-properties-system.c | 48 -
hw/core/qdev-properties.c| 62
3 files changed, 69 insertions(+), 69 deletions(-)
diff --git a/hw/core/qdev-prop-internal.h b/hw/core/qdev-prop-internal.h
index 6f17ddf271..740a5e530b 100644
--- a/hw/core/qdev-prop-internal.h
+++ b/hw/core/qdev-prop-internal.h
@@ -8,22 +8,22 @@
#ifndef HW_CORE_QDEV_PROP_INTERNAL_H
#define HW_CORE_QDEV_PROP_INTERNAL_H
-void qdev_propinfo_get_enum(Object *obj, Visitor *v, const char *name,
-void *opaque, Error **errp);
-void qdev_propinfo_set_enum(Object *obj, Visitor *v, const char *name,
-void *opaque, Error **errp);
+void field_prop_get_enum(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp);
+void field_prop_set_enum(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp);
-void qdev_propinfo_set_default_value_enum(ObjectProperty *op,
- const Property *prop);
-void qdev_propinfo_set_default_value_int(ObjectProperty *op,
- const Property *prop);
-void qdev_propinfo_set_default_value_uint(ObjectProperty *op,
- const Property *prop);
+void field_prop_set_default_value_enum(ObjectProperty *op,
+ const Property *prop);
+void field_prop_set_default_value_int(ObjectProperty *op,
+ const Property *prop);
+void field_prop_set_default_value_uint(ObjectProperty *op,
+ const Property *prop);
-void qdev_propinfo_get_int32(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp);
-void qdev_propinfo_get_size32(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp);
+void field_prop_get_int32(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp);
+void field_prop_get_size32(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp);
/**
* object_property_add_field: Add a field property to an object instance
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index f31aea3de1..590c5f3d97 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -537,9 +537,9 @@ QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int));
const PropertyInfo qdev_prop_losttickpolicy = {
.name = "LostTickPolicy",
.enum_table = &LostTickPolicy_lookup,
-.get = qdev_propinfo_get_enum,
-.set = qdev_propinfo_set_enum,
-.set_default_value = qdev_propinfo_set_default_value_enum,
+.get = field_prop_get_enum,
+.set = field_prop_set_enum,
+.set_default_value = field_prop_set_default_value_enum,
};
/* --- blocksize --- */
@@ -568,9 +568,9 @@ const PropertyInfo qdev_prop_blocksize = {
.name = "size",
.description = "A power of two between " MIN_BLOCK_SIZE_STR
" and " MAX_BLOCK_SIZE_STR,
-.get = qdev_propinfo_get_size32,
+.get = field_prop_get_size32,
.set = set_blocksize,
-.set_default_value = qdev_propinfo_set_default_value_uint,
+.set_default_value = field_prop_set_default_value_uint,
};
/* --- Block device error handling policy --- */
@@ -582,9 +582,9 @@ const PropertyInfo qdev_prop_blockdev_on_error = {
.description = "Error handling policy, "
"report/ignore/enospc/stop/auto",
.enum_table = &BlockdevOnError_lookup,
-.get = qdev_propinfo_get_enum,
-.set = qdev_propinfo_set_enum,
-.set_default_value = qdev_propinfo_set_default_value_enum,
+.get = field_prop_get_enum,
+.set = field_prop_set_enum,
+.set_default_value = field_prop_set_default_value_enum,
};
/* --- BIOS CHS translation */
@@ -596,9 +596,9 @@ const PropertyInfo qdev_prop_bios_chs_trans = {
.description = "Logical CHS translation algorithm, "
"auto/none/lba/large/rechs",
.enum_table = &BiosAtaTranslation_lookup,
-.get = qdev_propinfo_get_enum,
-.set = qdev_propinfo_set_enum,
-.set_default_value = qdev_propinfo_set_default_value_enum,
+.get = field_prop_get_enum,
+.set = field_prop_set_enum,
+.set_default_value = field_prop_set_default_value_enum,
};
/* --- FDC default drive types */
@@ -608,9 +608,9 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
.description = "FDC drive type, "
[PATCH v4 29/32] qdev: Rename qdev_prop_* to prop_info_*
The basic property types in qdev-properties.c are not going to be
qdev-specific anymore. Rename the variables to prop_info_*.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Redone patch after moving UUID property to qdev-properties-system.c
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Peter Maydell
Cc: Yoshinori Sato
Cc: Dmitry Fleytman
Cc: Jason Wang
Cc: Mark Cave-Ayland
Cc: Artyom Tarasenko
Cc: qemu-devel@nongnu.org
Cc: qemu-...@nongnu.org
---
include/hw/qdev-properties.h | 62 ++--
hw/core/qdev-properties.c| 36 ++---
hw/intc/arm_gicv3_common.c | 2 +-
hw/intc/rx_icu.c | 4 +--
hw/misc/arm_sysctl.c | 4 +--
hw/net/e1000e.c | 6 ++--
target/arm/cpu.c | 2 +-
target/sparc/cpu.c | 2 +-
8 files changed, 59 insertions(+), 59 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 68e544708b..90222822f1 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -44,22 +44,22 @@ struct PropertyInfo {
/*** qdev-properties.c ***/
-extern const PropertyInfo qdev_prop_bit;
-extern const PropertyInfo qdev_prop_bit64;
-extern const PropertyInfo qdev_prop_bool;
-extern const PropertyInfo qdev_prop_enum;
-extern const PropertyInfo qdev_prop_uint8;
-extern const PropertyInfo qdev_prop_uint16;
-extern const PropertyInfo qdev_prop_uint32;
-extern const PropertyInfo qdev_prop_int32;
-extern const PropertyInfo qdev_prop_uint64;
-extern const PropertyInfo qdev_prop_int64;
-extern const PropertyInfo qdev_prop_size;
-extern const PropertyInfo qdev_prop_string;
-extern const PropertyInfo qdev_prop_on_off_auto;
-extern const PropertyInfo qdev_prop_size32;
-extern const PropertyInfo qdev_prop_arraylen;
-extern const PropertyInfo qdev_prop_link;
+extern const PropertyInfo prop_info_bit;
+extern const PropertyInfo prop_info_bit64;
+extern const PropertyInfo prop_info_bool;
+extern const PropertyInfo prop_info_enum;
+extern const PropertyInfo prop_info_uint8;
+extern const PropertyInfo prop_info_uint16;
+extern const PropertyInfo prop_info_uint32;
+extern const PropertyInfo prop_info_int32;
+extern const PropertyInfo prop_info_uint64;
+extern const PropertyInfo prop_info_int64;
+extern const PropertyInfo prop_info_size;
+extern const PropertyInfo prop_info_string;
+extern const PropertyInfo prop_info_on_off_auto;
+extern const PropertyInfo prop_info_size32;
+extern const PropertyInfo prop_info_arraylen;
+extern const PropertyInfo prop_info_link;
#define DEFINE_PROP(_name, _state, _field, _prop, _type, ...) { \
.name = (_name),\
@@ -78,7 +78,7 @@ extern const PropertyInfo qdev_prop_link;
DEFINE_PROP(_name, _state, _field, _prop, _type)
#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) \
-DEFINE_PROP(_name, _state, _field, qdev_prop_bit, uint32_t, \
+DEFINE_PROP(_name, _state, _field, prop_info_bit, uint32_t, \
.bitnr = (_bit), \
.set_default = true,\
.defval.u= (bool)_defval)
@@ -92,13 +92,13 @@ extern const PropertyInfo qdev_prop_link;
DEFINE_PROP(_name, _state, _field, _prop, _type)
#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) \
-DEFINE_PROP(_name, _state, _field, qdev_prop_bit64, uint64_t, \
+DEFINE_PROP(_name, _state, _field, prop_info_bit64, uint64_t, \
.bitnr= (_bit), \
.set_default = true, \
.defval.u = (bool)_defval)
#define DEFINE_PROP_BOOL(_name, _state, _field, _defval) \
-DEFINE_PROP(_name, _state, _field, qdev_prop_bool, bool, \
+DEFINE_PROP(_name, _state, _field, prop_info_bool, bool, \
.set_default = true, \
.defval.u= (bool)_defval)
@@ -131,7 +131,7 @@ extern const PropertyInfo qdev_prop_link;
#define DEFINE_PROP_ARRAY(_name, _state, _field, \
_arrayfield, _arrayprop, _arraytype) \
DEFINE_PROP((PROP_ARRAY_LEN_PREFIX _name), \
-_state, _field, qdev_prop_arraylen, uint32_t, \
+_state, _field, prop_info_arraylen, uint32_t, \
.set_default = true, \
.defval.u = 0, \
.arrayinfo = &(_arrayprop),\
@@ -139,29 +139,29 @@ extern const PropertyInfo qdev_prop_link;
.arrayoffset = offsetof(_state, _arrayfield))
#define DEFINE_PROP_LINK(_name, _state, _field, _type, _ptr_type) \
-DEFINE_PROP(_name, _state, _field, qdev_prop_link, _ptr_type, \
+DEFINE_PROP(_name, _state, _field, prop_info_link, _ptr_type, \
.link
[PATCH v4 21/32] qdev: Add name argument to PropertyInfo.create method
This will make it easier to remove the Property.name field in the
future.
Signed-off-by: Eduardo Habkost
---
This is a new patch added in series v2
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
hw/core/qdev-properties.c| 7 ---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 476737b9da..ab9c538ba4 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -34,7 +34,7 @@ struct PropertyInfo {
const QEnumLookup *enum_table;
int (*print)(Object *obj, Property *prop, char *dest, size_t len);
void (*set_default_value)(ObjectProperty *op, const Property *prop);
-void (*create)(ObjectClass *oc, Property *prop);
+void (*create)(ObjectClass *oc, const char *name, Property *prop);
ObjectPropertyAccessor *get;
ObjectPropertyAccessor *set;
ObjectPropertyRelease *release;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 457c7fe4ba..c68a20695d 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -851,9 +851,10 @@ const PropertyInfo qdev_prop_size = {
/* --- object link property --- */
-static void create_link_property(ObjectClass *oc, Property *prop)
+static void create_link_property(ObjectClass *oc, const char *name,
+ Property *prop)
{
-object_class_property_add_link(oc, prop->name, prop->link_type,
+object_class_property_add_link(oc, name, prop->link_type,
prop->offset,
qdev_prop_allow_set_link_before_realize,
OBJ_PROP_LINK_STRONG);
@@ -893,7 +894,7 @@ static void qdev_class_add_property(DeviceClass *klass,
const char *name,
ObjectClass *oc = OBJECT_CLASS(klass);
if (prop->info->create) {
-prop->info->create(oc, prop);
+prop->info->create(oc, name, prop);
} else {
ObjectProperty *op;
--
2.28.0
[PATCH v4 28/32] qdev: Move qdev_prop_tpm declaration to tpm_prop.h
Move the variable declaration close to the macro that uses it. Reviewed-by: Marc-André Lureau Reviewed-by: Stefan Berger Signed-off-by: Eduardo Habkost --- Cc: Stefan Berger Cc: Paolo Bonzini Cc: "Daniel P. Berrangé" Cc: Eduardo Habkost Cc: qemu-devel@nongnu.org --- hw/tpm/tpm_prop.h| 2 ++ include/hw/qdev-properties.h | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/tpm/tpm_prop.h b/hw/tpm/tpm_prop.h index d19e40c112..bbd4225d66 100644 --- a/hw/tpm/tpm_prop.h +++ b/hw/tpm/tpm_prop.h @@ -25,6 +25,8 @@ #include "sysemu/tpm_backend.h" #include "hw/qdev-properties.h" +extern const PropertyInfo qdev_prop_tpm; + #define DEFINE_PROP_TPMBE(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *) diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index aae882317a..68e544708b 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -56,7 +56,6 @@ extern const PropertyInfo qdev_prop_uint64; extern const PropertyInfo qdev_prop_int64; extern const PropertyInfo qdev_prop_size; extern const PropertyInfo qdev_prop_string; -extern const PropertyInfo qdev_prop_tpm; extern const PropertyInfo qdev_prop_on_off_auto; extern const PropertyInfo qdev_prop_size32; extern const PropertyInfo qdev_prop_arraylen; -- 2.28.0
[PATCH v4 25/32] qdev: Make qdev_class_add_property() more flexible
Support Property.set_default and PropertyInfo.description even if
PropertyInfo.create is set.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Patch redone after changes in the previous patches in the
series
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 3bb05e7d0d..fcda0c8f4b 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -863,24 +863,25 @@ static void qdev_class_add_property(DeviceClass *klass,
const char *name,
Property *prop)
{
ObjectClass *oc = OBJECT_CLASS(klass);
+ObjectProperty *op;
if (prop->info->create) {
-prop->info->create(oc, name, prop);
+op = prop->info->create(oc, name, prop);
} else {
-ObjectProperty *op;
-
op = object_class_property_add(oc,
name, prop->info->name,
field_prop_getter(prop->info),
field_prop_setter(prop->info),
prop->info->release,
prop);
-if (prop->set_default) {
-prop->info->set_default_value(op, prop);
-}
}
-object_class_property_set_description(oc, name,
- prop->info->description);
+if (prop->set_default) {
+prop->info->set_default_value(op, prop);
+}
+if (prop->info->description) {
+object_class_property_set_description(oc, name,
+ prop->info->description);
+}
}
/**
--
2.28.0
[PATCH v4 26/32] qdev: Separate generic and device-specific property registration
qdev_class_add_property() and qdev_property_add_static() will
have code that's specific for device types.
object_class_property_add_field_static() and
object_property_add_field() will be generic and part of the QOM
field property API. Note that the new functions have a `name`
parameter because the plan is to eventually get rid of the
Property.name field.
The declarations for the new functions are being added to
qdev-properties-internal.h, but they will be moved to a QOM
header later.
Signed-off-by: Eduardo Habkost
---
Changes v2 -> v3:
* Re-added array-based array registration function, named
as object_class_add_field_properties()
* Renamed object_class_property_add_field() to
object_class_property_add_field_static(), to indicate that
the function expect the Property argument to have static life
time.
* Keep all new functions as internal API by now,
until we decide what's going to be the preferred API for
registering class field properties.
Changes v1 -> v2:
* Patch redone after changes in previous patches in the series
* Rename new functions to object*_property_add_field()
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-prop-internal.h | 42
hw/core/qdev-properties.c| 37 ---
2 files changed, 71 insertions(+), 8 deletions(-)
diff --git a/hw/core/qdev-prop-internal.h b/hw/core/qdev-prop-internal.h
index d7b77844fe..6f17ddf271 100644
--- a/hw/core/qdev-prop-internal.h
+++ b/hw/core/qdev-prop-internal.h
@@ -25,4 +25,46 @@ void qdev_propinfo_get_int32(Object *obj, Visitor *v, const
char *name,
void qdev_propinfo_get_size32(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp);
+/**
+ * object_property_add_field: Add a field property to an object instance
+ * @obj: object instance
+ * @name: property name
+ * @prop: property definition
+ *
+ * This function should not be used in new code. Please add class properties
+ * instead, using object_class_add_field().
+ */
+ObjectProperty *
+object_property_add_field(Object *obj, const char *name,
+ Property *prop);
+
+/**
+ * object_class_property_add_field_static: Add a field property to object class
+ * @oc: object class
+ * @name: property name
+ * @prop: property definition
+ *
+ * Add a field property to an object class. A field property is
+ * a property that will change a field at a specific offset of the
+ * object instance struct.
+ *
+ * *@prop must have static life time.
+ */
+ObjectProperty *
+object_class_property_add_field_static(ObjectClass *oc, const char *name,
+ Property *prop);
+
+/**
+ * object_class_add_field_properties: Add field properties from array to a
class
+ * @oc: object class
+ * @props: array of property definitions
+ *
+ * Register an array of field properties to a class, using
+ * object_class_property_add_field_static() for each array element.
+ *
+ * The array at @props must end with DEFINE_PROP_END_OF_LIST(), and
+ * must have static life time.
+ */
+void object_class_add_field_properties(ObjectClass *oc, Property *props);
+
#endif
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index fcda0c8f4b..8436b60ec4 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -835,20 +835,21 @@ const PropertyInfo qdev_prop_link = {
.create = create_link_property,
};
-void qdev_property_add_static(DeviceState *dev, Property *prop)
+ObjectProperty *
+object_property_add_field(Object *obj, const char *name,
+ Property *prop)
{
-Object *obj = OBJECT(dev);
ObjectProperty *op;
assert(!prop->info->create);
-op = object_property_add(obj, prop->name, prop->info->name,
+op = object_property_add(obj, name, prop->info->name,
field_prop_getter(prop->info),
field_prop_setter(prop->info),
prop->info->release,
prop);
-object_property_set_description(obj, prop->name,
+object_property_set_description(obj, name,
prop->info->description);
if (prop->set_default) {
@@ -857,12 +858,14 @@ void qdev_property_add_static(DeviceState *dev, Property
*prop)
op->init(obj, op);
}
}
+
+return op;
}
-static void qdev_class_add_property(DeviceClass *klass, const char *name,
-Property *prop)
+ObjectProperty *
+object_class_property_add_field_static(ObjectClass *oc, const char *name,
+ Property *prop)
{
-ObjectClass *oc = OBJECT_CLASS(klass);
ObjectProperty *op;
if (prop->info->create) {
@@ -882,6 +885,22 @@ static void qdev_class_add_property(DeviceClass *klass,
const char *name,
object_class_property_set_descript
[PATCH v4 24/32] qdev: Make PropertyInfo.create return ObjectProperty*
Returning ObjectProperty* will be useful for new property
registration code that will add additional callbacks
to ObjectProperty after registering it.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Redone patch on top of additional changes in series v2
* Commit message reword
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/hw/qdev-properties.h | 3 ++-
hw/core/qdev-properties.c| 12 ++--
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index ab9c538ba4..aae882317a 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -34,7 +34,8 @@ struct PropertyInfo {
const QEnumLookup *enum_table;
int (*print)(Object *obj, Property *prop, char *dest, size_t len);
void (*set_default_value)(ObjectProperty *op, const Property *prop);
-void (*create)(ObjectClass *oc, const char *name, Property *prop);
+ObjectProperty *(*create)(ObjectClass *oc, const char *name,
+ Property *prop);
ObjectPropertyAccessor *get;
ObjectPropertyAccessor *set;
ObjectPropertyRelease *release;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 92f48ecbf2..3bb05e7d0d 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -821,13 +821,13 @@ const PropertyInfo qdev_prop_size = {
/* --- object link property --- */
-static void create_link_property(ObjectClass *oc, const char *name,
- Property *prop)
+static ObjectProperty *create_link_property(ObjectClass *oc, const char *name,
+Property *prop)
{
-object_class_property_add_link(oc, name, prop->link_type,
- prop->offset,
- qdev_prop_allow_set_link_before_realize,
- OBJ_PROP_LINK_STRONG);
+return object_class_property_add_link(oc, name, prop->link_type,
+ prop->offset,
+
qdev_prop_allow_set_link_before_realize,
+ OBJ_PROP_LINK_STRONG);
}
const PropertyInfo qdev_prop_link = {
--
2.28.0
[PATCH v4 20/32] qdev: Add name parameter to qdev_class_add_property()
This will make it easier to remove Property.name in the future.
Signed-off-by: Eduardo Habkost
---
This is a new patch added in series v2
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 50734a1cd4..457c7fe4ba 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -887,7 +887,8 @@ void qdev_property_add_static(DeviceState *dev, Property
*prop)
}
}
-static void qdev_class_add_property(DeviceClass *klass, Property *prop)
+static void qdev_class_add_property(DeviceClass *klass, const char *name,
+Property *prop)
{
ObjectClass *oc = OBJECT_CLASS(klass);
@@ -897,7 +898,7 @@ static void qdev_class_add_property(DeviceClass *klass,
Property *prop)
ObjectProperty *op;
op = object_class_property_add(oc,
- prop->name, prop->info->name,
+ name, prop->info->name,
prop->info->get, prop->info->set,
prop->info->release,
prop);
@@ -905,7 +906,7 @@ static void qdev_class_add_property(DeviceClass *klass,
Property *prop)
prop->info->set_default_value(op, prop);
}
}
-object_class_property_set_description(oc, prop->name,
+object_class_property_set_description(oc, name,
prop->info->description);
}
@@ -962,7 +963,7 @@ void device_class_set_props(DeviceClass *dc, Property
*props)
dc->props_ = props;
for (prop = props; prop && prop->name; prop++) {
qdev_class_add_legacy_property(dc, prop);
-qdev_class_add_property(dc, prop);
+qdev_class_add_property(dc, prop->name, prop);
}
}
--
2.28.0
[PATCH v4 23/32] qdev: Move dev->realized check to qdev_property_set()
Every single qdev property setter function manually checks
dev->realized. We can just check dev->realized inside
qdev_property_set() instead.
The check is being added as a separate function
(qdev_prop_allow_set()) because it will become a callback later.
Reviewed-by: Stefan Berger
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Removed unused variable at xen_block_set_vdev()
* Redone patch after changes in the previous patches in the
series
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Thomas Huth
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: Mark Cave-Ayland
Cc: Artyom Tarasenko
Cc: qemu-devel@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
---
backends/tpm/tpm_util.c | 6 --
hw/block/xen-block.c | 6 --
hw/core/qdev-properties-system.c | 70 --
hw/core/qdev-properties.c| 100 ++-
hw/s390x/css.c | 6 --
hw/s390x/s390-pci-bus.c | 6 --
hw/vfio/pci-quirks.c | 6 --
target/sparc/cpu.c | 6 --
8 files changed, 18 insertions(+), 188 deletions(-)
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index a5d997e7dc..39b45fa46d 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -46,16 +46,10 @@ static void get_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, &str, errp)) {
return;
}
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 905e4acd97..bd1aef63a7 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -395,17 +395,11 @@ static int vbd_name_to_disk(const char *name, const char
**endp,
static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
char *str, *p;
const char *end;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, &str, errp)) {
return;
}
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 42529c3b65..f31aea3de1 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -94,11 +94,6 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
bool blk_created = false;
int ret;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, &str, errp)) {
return;
}
@@ -230,17 +225,11 @@ static void get_chr(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
CharBackend *be = qdev_get_prop_ptr(obj, prop);
Chardev *s;
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, &str, errp)) {
return;
}
@@ -311,18 +300,12 @@ static void get_mac(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
MACAddr *mac = qdev_get_prop_ptr(obj, prop);
int i, pos;
char *str;
const char *p;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, &str, errp)) {
return;
}
@@ -390,7 +373,6 @@ static void get_netdev(Object *obj, Visitor *v, const char
*name,
static void set_netdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
NICPeers *peers_ptr = qdev_get_prop_ptr(obj, prop);
NetClientState **ncs = peers_ptr->ncs;
@@ -398,11 +380,6 @@ static void set_netdev(Object *obj, Visitor *v, const char
*name,
int queues, err = 0, i = 0;
char *str;
-if (dev->reali
[PATCH v4 19/32] qdev: Avoid using prop->name unnecessarily
We already get the property name as argument to the property
getter and setters, we don't need to use prop->name. This will
make it easier to remove the Property.name field in the future.
Reviewed-by: Stefan Berger
Signed-off-by: Eduardo Habkost
---
This is a new patch added in series v2
---
Cc: Stefan Berger
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
backends/tpm/tpm_util.c | 2 +-
hw/core/qdev-properties-system.c | 14 +++---
hw/core/qdev-properties.c| 4 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 3973105658..a5d997e7dc 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -63,7 +63,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
s = qemu_find_tpm_be(str);
if (s == NULL) {
error_setg(errp, "Property '%s.%s' can't find value '%s'",
- object_get_typename(obj), prop->name, str);
+ object_get_typename(obj), name, str);
} else if (tpm_backend_init(s, TPM_IF(obj), errp) == 0) {
*be = s; /* weak reference, avoid cyclic ref */
}
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 9cf9bcb39d..42529c3b65 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -141,7 +141,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
}
if (!blk) {
error_setg(errp, "Property '%s.%s' can't find value '%s'",
- object_get_typename(OBJECT(dev)), prop->name, str);
+ object_get_typename(OBJECT(dev)), name, str);
goto fail;
}
if (blk_attach_dev(blk, dev) < 0) {
@@ -262,10 +262,10 @@ static void set_chr(Object *obj, Visitor *v, const char
*name, void *opaque,
s = qemu_chr_find(str);
if (s == NULL) {
error_setg(errp, "Property '%s.%s' can't find value '%s'",
- object_get_typename(obj), prop->name, str);
+ object_get_typename(obj), name, str);
} else if (!qemu_chr_fe_init(be, s, errp)) {
error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
- object_get_typename(obj), prop->name, str);
+ object_get_typename(obj), name, str);
}
g_free(str);
}
@@ -966,7 +966,7 @@ static void get_prop_pcielinkspeed(Object *obj, Visitor *v,
const char *name,
abort();
}
-visit_type_enum(v, prop->name, &speed, prop->info->enum_table, errp);
+visit_type_enum(v, name, &speed, prop->info->enum_table, errp);
}
static void set_prop_pcielinkspeed(Object *obj, Visitor *v, const char *name,
@@ -982,7 +982,7 @@ static void set_prop_pcielinkspeed(Object *obj, Visitor *v,
const char *name,
return;
}
-if (!visit_type_enum(v, prop->name, &speed, prop->info->enum_table,
+if (!visit_type_enum(v, name, &speed, prop->info->enum_table,
errp)) {
return;
}
@@ -1051,7 +1051,7 @@ static void get_prop_pcielinkwidth(Object *obj, Visitor
*v, const char *name,
abort();
}
-visit_type_enum(v, prop->name, &width, prop->info->enum_table, errp);
+visit_type_enum(v, name, &width, prop->info->enum_table, errp);
}
static void set_prop_pcielinkwidth(Object *obj, Visitor *v, const char *name,
@@ -1067,7 +1067,7 @@ static void set_prop_pcielinkwidth(Object *obj, Visitor
*v, const char *name,
return;
}
-if (!visit_type_enum(v, prop->name, &width, prop->info->enum_table,
+if (!visit_type_enum(v, name, &width, prop->info->enum_table,
errp)) {
return;
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7495798a66..50734a1cd4 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -50,7 +50,7 @@ void qdev_propinfo_get_enum(Object *obj, Visitor *v, const
char *name,
Property *prop = opaque;
int *ptr = qdev_get_prop_ptr(obj, prop);
-visit_type_enum(v, prop->name, ptr, prop->info->enum_table, errp);
+visit_type_enum(v, name, ptr, prop->info->enum_table, errp);
}
void qdev_propinfo_set_enum(Object *obj, Visitor *v, const char *name,
@@ -65,7 +65,7 @@ void qdev_propinfo_set_enum(Object *obj, Visitor *v, const
char *name,
return;
}
-visit_type_enum(v, prop->name, ptr, prop->info->enum_table, errp);
+visit_type_enum(v, name, ptr, prop->info->enum_table, errp);
}
void qdev_propinfo_set_default_value_enum(ObjectProperty *op,
--
2.28.0
[PATCH v4 16/32] qdev: Reuse DEFINE_PROP in all DEFINE_PROP_* macros
Instead of duplicating the code that sets name, info, offset,
and does type checking, make DEFINE_PROP accept a variable number
of arguments and reuse it in all DEFINE_PROP_* macros.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Redone after UUID property was moved
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/hw/qdev-properties-system.h | 19 ++---
include/hw/qdev-properties.h| 114 ++--
2 files changed, 46 insertions(+), 87 deletions(-)
diff --git a/include/hw/qdev-properties-system.h
b/include/hw/qdev-properties-system.h
index 29529dc999..0ac327ae60 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -63,22 +63,15 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pcie_link_width, \
PCIExpLinkWidth)
-#define DEFINE_PROP_UUID(_name, _state, _field) { \
-.name = (_name), \
-.info = &qdev_prop_uuid, \
-.offset= offsetof(_state, _field) \
-+ type_check(QemuUUID, typeof_field(_state, _field)), \
-.set_default = true, \
-}
+#define DEFINE_PROP_UUID(_name, _state, _field) \
+DEFINE_PROP(_name, _state, _field, qdev_prop_uuid, QemuUUID, \
+.set_default = true)
+
#define DEFINE_PROP_AUDIODEV(_n, _s, _f) \
DEFINE_PROP(_n, _s, _f, qdev_prop_audiodev, QEMUSoundCard)
-#define DEFINE_PROP_UUID_NODEFAULT(_name, _state, _field) {\
-.name = (_name), \
-.info = &qdev_prop_uuid, \
-.offset= offsetof(_state, _field) \
-+ type_check(QemuUUID, typeof_field(_state, _field)), \
-}
+#define DEFINE_PROP_UUID_NODEFAULT(_name, _state, _field) \
+DEFINE_PROP(_name, _state, _field, qdev_prop_uuid, QemuUUID)
#endif
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index d35d4aae84..1b58e4f922 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -61,73 +61,46 @@ extern const PropertyInfo qdev_prop_size32;
extern const PropertyInfo qdev_prop_arraylen;
extern const PropertyInfo qdev_prop_link;
-#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
+#define DEFINE_PROP(_name, _state, _field, _prop, _type, ...) { \
.name = (_name),\
.info = &(_prop), \
.offset= offsetof(_state, _field)\
+ type_check(_type, typeof_field(_state, _field)), \
+__VA_ARGS__ \
}
-#define DEFINE_PROP_SIGNED(_name, _state, _field, _defval, _prop, _type) { \
-.name = (_name), \
-.info = &(_prop), \
-.offset= offsetof(_state, _field) \
-+ type_check(_type,typeof_field(_state, _field)), \
-.set_default = true,\
-.defval.i = (_type)_defval,\
-}
+#define DEFINE_PROP_SIGNED(_name, _state, _field, _defval, _prop, _type) \
+DEFINE_PROP(_name, _state, _field, _prop, _type, \
+.set_default = true, \
+.defval.i= (_type)_defval)
-#define DEFINE_PROP_SIGNED_NODEFAULT(_name, _state, _field, _prop, _type) { \
-.name = (_name), \
-.info = &(_prop), \
-.offset= offsetof(_state, _field) \
-+ type_check(_type, typeof_field(_state, _field)), \
-}
+#define DEFINE_PROP_SIGNED_NODEFAULT(_name, _state, _field, _prop, _type) \
+DEFINE_PROP(_name, _state, _field, _prop, _type)
-#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
-.name = (_name),\
-.info = &(qdev_prop_bit), \
-.bitnr= (_bit), \
-.offset= offsetof(_state, _field)\
-+ type_check(uint32_t,typeof_field(_state, _field)), \
-.set_default = true, \
-.defval.u = (bool)_defval, \
-}
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) \
+DEFINE_PROP(_name, _state, _field, qdev_prop_bit, uint32_t, \
+
[PATCH v4 10/32] qdev: Make qdev_find_global_prop() get Object* argument
Make the code more generic and not specific to TYPE_DEVICE.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
hw/core/qdev-properties-system.c | 2 +-
hw/core/qdev-properties.c| 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0b92cfc761..7620095fed 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -305,7 +305,7 @@ void qdev_prop_set_enum(DeviceState *dev, const char *name,
int value);
void *qdev_get_prop_ptr(Object *obj, Property *prop);
void qdev_prop_register_global(GlobalProperty *prop);
-const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
+const GlobalProperty *qdev_find_global_prop(Object *obj,
const char *name);
int qdev_prop_check_globals(void);
void qdev_prop_set_globals(DeviceState *dev);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 9ac9b95852..57e63c6949 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -36,7 +36,7 @@ static bool check_prop_still_unset(DeviceState *dev, const
char *name,
const void *old_val, const char *new_val,
Error **errp)
{
-const GlobalProperty *prop = qdev_find_global_prop(dev, name);
+const GlobalProperty *prop = qdev_find_global_prop(OBJECT(dev), name);
if (!old_val) {
return true;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 0a54a922c8..41482d83d1 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -818,7 +818,7 @@ void qdev_prop_register_global(GlobalProperty *prop)
g_ptr_array_add(global_props(), prop);
}
-const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
+const GlobalProperty *qdev_find_global_prop(Object *obj,
const char *name)
{
GPtrArray *props = global_props();
@@ -827,7 +827,7 @@ const GlobalProperty *qdev_find_global_prop(DeviceState
*dev,
for (i = 0; i < props->len; i++) {
p = g_ptr_array_index(props, i);
-if (object_dynamic_cast(OBJECT(dev), p->driver)
+if (object_dynamic_cast(obj, p->driver)
&& !strcmp(p->property, name)) {
return p;
}
--
2.28.0
[PATCH v4 22/32] qdev: Wrap getters and setters in separate helpers
We'll add extra code to the qdev property getters and setters, so
add wrapper functions where additional actions can be performed.
The new functions have a "field_prop_" prefix instead of "qdev_"
because the code will eventually be moved outside
qdev-properties.c, to common QOM code.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* Redone after changes in previous patches in the series
* Renamed functions from static_prop_* to field_prop_*
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties.c | 44 +++
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index c68a20695d..b924f13d58 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -44,6 +44,40 @@ void *qdev_get_prop_ptr(Object *obj, Property *prop)
return ptr;
}
+static void field_prop_get(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+Property *prop = opaque;
+return prop->info->get(obj, v, name, opaque, errp);
+}
+
+/**
+ * field_prop_getter: Return getter function to be used for property
+ *
+ * Return value can be NULL if @info has no getter function.
+ */
+static ObjectPropertyAccessor *field_prop_getter(const PropertyInfo *info)
+{
+return info->get ? field_prop_get : NULL;
+}
+
+static void field_prop_set(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+Property *prop = opaque;
+return prop->info->set(obj, v, name, opaque, errp);
+}
+
+/**
+ * field_prop_setter: Return setter function to be used for property
+ *
+ * Return value can be NULL if @info has not setter function.
+ */
+static ObjectPropertyAccessor *field_prop_setter(const PropertyInfo *info)
+{
+return info->set ? field_prop_set : NULL;
+}
+
void qdev_propinfo_get_enum(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -630,8 +664,8 @@ static void set_prop_arraylen(Object *obj, Visitor *v,
const char *name,
assert(qdev_get_prop_ptr(obj, &arrayprop->prop) == eltptr);
object_property_add(obj, propname,
arrayprop->prop.info->name,
-arrayprop->prop.info->get,
-arrayprop->prop.info->set,
+field_prop_getter(arrayprop->prop.info),
+field_prop_setter(arrayprop->prop.info),
array_element_release,
arrayprop);
}
@@ -873,7 +907,8 @@ void qdev_property_add_static(DeviceState *dev, Property
*prop)
assert(!prop->info->create);
op = object_property_add(obj, prop->name, prop->info->name,
- prop->info->get, prop->info->set,
+ field_prop_getter(prop->info),
+ field_prop_setter(prop->info),
prop->info->release,
prop);
@@ -900,7 +935,8 @@ static void qdev_class_add_property(DeviceClass *klass,
const char *name,
op = object_class_property_add(oc,
name, prop->info->name,
- prop->info->get, prop->info->set,
+ field_prop_getter(prop->info),
+ field_prop_setter(prop->info),
prop->info->release,
prop);
if (prop->set_default) {
--
2.28.0
[PATCH v4 11/32] qdev: Make check_prop_still_unset() get Object* argument
Make the code more generic and not specific to TYPE_DEVICE.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties-system.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 57e63c6949..58bb129bbe 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -32,11 +32,11 @@
#include "hw/pci/pci.h"
#include "util/block-helpers.h"
-static bool check_prop_still_unset(DeviceState *dev, const char *name,
+static bool check_prop_still_unset(Object *obj, const char *name,
const void *old_val, const char *new_val,
Error **errp)
{
-const GlobalProperty *prop = qdev_find_global_prop(OBJECT(dev), name);
+const GlobalProperty *prop = qdev_find_global_prop(obj, name);
if (!old_val) {
return true;
@@ -105,7 +105,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
* TODO Should this really be an error? If no, the old value
* needs to be released before we store the new one.
*/
-if (!check_prop_still_unset(dev, name, *ptr, str, errp)) {
+if (!check_prop_still_unset(obj, name, *ptr, str, errp)) {
return;
}
@@ -247,7 +247,7 @@ static void set_chr(Object *obj, Visitor *v, const char
*name, void *opaque,
* TODO Should this really be an error? If no, the old value
* needs to be released before we store the new one.
*/
-if (!check_prop_still_unset(dev, name, be->chr, str, errp)) {
+if (!check_prop_still_unset(obj, name, be->chr, str, errp)) {
return;
}
@@ -429,7 +429,7 @@ static void set_netdev(Object *obj, Visitor *v, const char
*name,
* TODO Should this really be an error? If no, the old value
* needs to be released before we store the new one.
*/
-if (!check_prop_still_unset(dev, name, ncs[i], str, errp)) {
+if (!check_prop_still_unset(obj, name, ncs[i], str, errp)) {
goto out;
}
--
2.28.0
[PATCH v4 15/32] qdev: Move softmmu properties to qdev-properties-system.h
Move the property types and property macros implemented in qdev-properties-system.c to a new qdev-properties-system.h header. Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Move UUID property type too, as it was moved to qdev-properties-system.c in the previous patch --- audio/audio.h | 1 + include/hw/block/block.h| 1 + include/hw/qdev-properties-system.h | 84 + include/hw/qdev-properties.h| 75 -- include/net/net.h | 1 + hw/acpi/vmgenid.c | 1 + hw/arm/pxa2xx.c | 1 + hw/arm/strongarm.c | 1 + hw/block/fdc.c | 1 + hw/block/m25p80.c | 1 + hw/block/nand.c | 1 + hw/block/onenand.c | 1 + hw/block/pflash_cfi01.c | 1 + hw/block/pflash_cfi02.c | 1 + hw/block/vhost-user-blk.c | 1 + hw/char/avr_usart.c | 1 + hw/char/bcm2835_aux.c | 1 + hw/char/cadence_uart.c | 1 + hw/char/cmsdk-apb-uart.c| 1 + hw/char/debugcon.c | 1 + hw/char/digic-uart.c| 1 + hw/char/escc.c | 1 + hw/char/etraxfs_ser.c | 1 + hw/char/exynos4210_uart.c | 1 + hw/char/grlib_apbuart.c | 1 + hw/char/ibex_uart.c | 1 + hw/char/imx_serial.c| 1 + hw/char/ipoctal232.c| 1 + hw/char/lm32_juart.c| 1 + hw/char/lm32_uart.c | 1 + hw/char/mcf_uart.c | 1 + hw/char/milkymist-uart.c| 1 + hw/char/nrf51_uart.c| 1 + hw/char/parallel.c | 1 + hw/char/pl011.c | 1 + hw/char/renesas_sci.c | 1 + hw/char/sclpconsole-lm.c| 1 + hw/char/sclpconsole.c | 1 + hw/char/serial-pci-multi.c | 1 + hw/char/serial.c| 1 + hw/char/spapr_vty.c | 1 + hw/char/stm32f2xx_usart.c | 1 + hw/char/terminal3270.c | 1 + hw/char/virtio-console.c| 1 + hw/char/xilinx_uartlite.c | 1 + hw/core/qdev-properties-system.c| 1 + hw/hyperv/vmbus.c | 1 + hw/i386/kvm/i8254.c | 1 + hw/ide/qdev.c | 1 + hw/ipmi/ipmi_bmc_extern.c | 1 + hw/ipmi/ipmi_bmc_sim.c | 1 + hw/misc/allwinner-sid.c | 1 + hw/misc/ivshmem.c | 1 + hw/misc/mac_via.c | 1 + hw/misc/sifive_u_otp.c | 1 + hw/net/rocker/rocker.c | 1 + hw/nvram/eeprom_at24c.c | 1 + hw/nvram/spapr_nvram.c | 1 + hw/pci-bridge/gen_pcie_root_port.c | 1 + hw/pci/pci.c| 1 + hw/ppc/pnv_pnor.c | 1 + hw/rdma/vmw/pvrdma_main.c | 1 + hw/rtc/mc146818rtc.c| 1 + hw/scsi/scsi-disk.c | 1 + hw/scsi/scsi-generic.c | 1 + hw/scsi/vhost-user-scsi.c | 1 + hw/sd/sd.c | 1 + hw/usb/ccid-card-passthru.c | 1 + hw/usb/dev-serial.c | 1 + hw/usb/redirect.c | 1 + hw/vfio/pci.c | 1 + hw/virtio/vhost-user-fs.c | 1 + hw/virtio/vhost-user-vsock.c| 1 + hw/virtio/virtio-iommu-pci.c| 1 + hw/xen/xen_pt.c | 1 + migration/migration.c | 1 + 76 files changed, 158 insertions(+), 75 deletions(-) create mode 100644 include/hw/qdev-properties-system.h diff --git a/audio/audio.h b/audio/audio.h index b883ebfb1f..21fe3226ae 100644 --- a/audio/audio.h +++ b/audio/audio.h @@ -28,6 +28,7 @@ #include "qemu/queue.h" #include "qapi/qapi-types-audio.h" #include "hw/qdev-properties.h" +#include "hw/qdev-properties-system.h" typedef void (*audio_callback_fn) (void *opaque, int avail); diff --git a/include/hw/block/block.h b/include/hw/block/block.h index 1e8b6253dd..c172cbe65f 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -13,6 +13,7 @@ #include "exec/hwaddr.h" #include "qapi/qapi-types-block-core.h" +#include "hw/qdev-properties-system.h" /* Configuration */ diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h new file mode 100644 index 00..29529dc999 --- /dev/null +++ b/include/hw/qdev-properties-system.h @@ -0,0 +1,84 @@ +#ifndef HW_QDEV_PROPERTIES_SYSTEM_H +#define HW_QDEV_PROPERTIES_SYSTEM_H + +#include "hw/qdev-properties.h" + +extern const PropertyInfo qdev_prop_chr; +extern const PropertyInfo qdev_prop_macaddr; +extern const PropertyInfo qdev_prop_reserved_region; +extern const PropertyInfo qdev_prop_multifd_compression; +extern const PropertyInfo qdev_p
[PATCH v4 17/32] sparc: Use DEFINE_PROP for nwindows property
Use the DEFINE_PROP macro (which will set extra fields in the
struct) instead of initializing a Property struct manually.
Signed-off-by: Eduardo Habkost
---
This is a new patch added in v2 of the series
---
Cc: Mark Cave-Ayland
Cc: Artyom Tarasenko
Cc: qemu-devel@nongnu.org
---
target/sparc/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 6a3299041f..92534bcd18 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -848,7 +848,8 @@ static Property sparc_cpu_properties[] = {
qdev_prop_uint64, target_ulong),
DEFINE_PROP_UINT32("fpu-version", SPARCCPU, env.def.fpu_version, 0),
DEFINE_PROP_UINT32("mmu-version", SPARCCPU, env.def.mmu_version, 0),
-{ .name = "nwindows", .info = &qdev_prop_nwindows },
+DEFINE_PROP("nwindows", SPARCCPU, env.def.nwindows,
+qdev_prop_nwindows, uint32_t),
DEFINE_PROP_END_OF_LIST()
};
--
2.28.0
[PATCH v4 14/32] qdev: Move UUID property to qdev-properties-system.c
Only softmmu code uses DEFINE_PROP_UUID, and it currently depends
on error_set_from_qdev_prop_error(). Move it to
qdev-properties-system.c to get out of our way when refactoring
the qdev property system.
We can eventually move it back to the core property system later,
after removing usage of error_set_from_qdev_prop_error().
Signed-off-by: Eduardo Habkost
---
This is a new patch added in series v2
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties-system.c | 57
hw/core/qdev-properties.c| 57
2 files changed, 57 insertions(+), 57 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 5796ed2619..7a9a1d6404 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -21,6 +21,7 @@
#include "qemu/ctype.h"
#include "qemu/cutils.h"
#include "qemu/units.h"
+#include "qemu/uuid.h"
#include "qemu/error-report.h"
#include "qdev-prop-internal.h"
@@ -1106,3 +1107,59 @@ const PropertyInfo qdev_prop_pcie_link_width = {
.set = set_prop_pcielinkwidth,
.set_default_value = qdev_propinfo_set_default_value_enum,
};
+
+/* --- UUID --- */
+
+static void get_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+Property *prop = opaque;
+QemuUUID *uuid = qdev_get_prop_ptr(obj, prop);
+char buffer[UUID_FMT_LEN + 1];
+char *p = buffer;
+
+qemu_uuid_unparse(uuid, buffer);
+
+visit_type_str(v, name, &p, errp);
+}
+
+#define UUID_VALUE_AUTO"auto"
+
+static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
+Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+QemuUUID *uuid = qdev_get_prop_ptr(obj, prop);
+char *str;
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+if (!visit_type_str(v, name, &str, errp)) {
+return;
+}
+
+if (!strcmp(str, UUID_VALUE_AUTO)) {
+qemu_uuid_generate(uuid);
+} else if (qemu_uuid_parse(str, uuid) < 0) {
+error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+}
+g_free(str);
+}
+
+static void set_default_uuid_auto(ObjectProperty *op, const Property *prop)
+{
+object_property_set_default_str(op, UUID_VALUE_AUTO);
+}
+
+const PropertyInfo qdev_prop_uuid = {
+.name = "str",
+.description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
+"\" for random value (default)",
+.get = get_uuid,
+.set = set_uuid,
+.set_default_value = set_default_uuid_auto,
+};
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 765e916c23..a2eaa43831 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -6,7 +6,6 @@
#include "qemu/ctype.h"
#include "qemu/error-report.h"
#include "qapi/visitor.h"
-#include "qemu/uuid.h"
#include "qemu/units.h"
#include "qemu/cutils.h"
#include "qdev-prop-internal.h"
@@ -544,62 +543,6 @@ const PropertyInfo qdev_prop_size32 = {
.set_default_value = qdev_propinfo_set_default_value_uint,
};
-/* --- UUID --- */
-
-static void get_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
- Error **errp)
-{
-Property *prop = opaque;
-QemuUUID *uuid = qdev_get_prop_ptr(obj, prop);
-char buffer[UUID_FMT_LEN + 1];
-char *p = buffer;
-
-qemu_uuid_unparse(uuid, buffer);
-
-visit_type_str(v, name, &p, errp);
-}
-
-#define UUID_VALUE_AUTO"auto"
-
-static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
-Error **errp)
-{
-DeviceState *dev = DEVICE(obj);
-Property *prop = opaque;
-QemuUUID *uuid = qdev_get_prop_ptr(obj, prop);
-char *str;
-
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
-if (!visit_type_str(v, name, &str, errp)) {
-return;
-}
-
-if (!strcmp(str, UUID_VALUE_AUTO)) {
-qemu_uuid_generate(uuid);
-} else if (qemu_uuid_parse(str, uuid) < 0) {
-error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
-}
-g_free(str);
-}
-
-static void set_default_uuid_auto(ObjectProperty *op, const Property *prop)
-{
-object_property_set_default_str(op, UUID_VALUE_AUTO);
-}
-
-const PropertyInfo qdev_prop_uuid = {
-.name = "str",
-.description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
-"\" for random value (default)",
-.get = get_uuid,
-.set = set_uuid,
-.set_default_value = set_default_uuid_auto,
-};
-
/* --- support for array properties --- */
/* Used as an opaque for the object properties we add for each
--
2.28.0
[PATCH v4 18/32] qdev: Get just property name at error_set_from_qdev_prop_error()
Replace `Property *prop` parameter with `char *name`, to reduce
dependency of getter and setter functions on the Property struct
(which will be changed in following patches).
Signed-off-by: Eduardo Habkost
---
This is a new patch added in series v2
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-devel@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
hw/core/qdev-properties-system.c | 12 ++--
hw/core/qdev-properties.c| 8
hw/s390x/css.c | 2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 1b58e4f922..476737b9da 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -201,7 +201,7 @@ const GlobalProperty *qdev_find_global_prop(Object *obj,
int qdev_prop_check_globals(void);
void qdev_prop_set_globals(DeviceState *dev);
void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
-Property *prop, const char *value);
+const char *name, const char *value);
/**
* qdev_property_add_static:
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index e2d523b27a..9cf9bcb39d 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -354,7 +354,7 @@ static void set_mac(Object *obj, Visitor *v, const char
*name, void *opaque,
return;
inval:
-error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, name, str);
g_free(str);
}
@@ -442,7 +442,7 @@ static void set_netdev(Object *obj, Visitor *v, const char
*name,
peers_ptr->queues = queues;
out:
-error_set_from_qdev_prop_error(errp, err, obj, prop, str);
+error_set_from_qdev_prop_error(errp, err, obj, name, str);
g_free(str);
}
@@ -494,7 +494,7 @@ static void set_audiodev(Object *obj, Visitor *v, const
char* name,
card->state = state;
out:
-error_set_from_qdev_prop_error(errp, err, obj, prop, str);
+error_set_from_qdev_prop_error(errp, err, obj, name, str);
g_free(str);
}
@@ -792,7 +792,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, const
char *name,
return;
invalid:
-error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, name, str);
g_free(str);
}
@@ -916,7 +916,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v,
const char *name,
return;
inval:
-error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, name, str);
g_free(str);
}
@@ -1146,7 +1146,7 @@ static void set_uuid(Object *obj, Visitor *v, const char
*name, void *opaque,
if (!strcmp(str, UUID_VALUE_AUTO)) {
qemu_uuid_generate(uuid);
} else if (qemu_uuid_parse(str, uuid) < 0) {
-error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, name, str);
}
g_free(str);
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index a2eaa43831..7495798a66 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -679,21 +679,21 @@ static Property *qdev_prop_find(DeviceState *dev, const
char *name)
}
void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
-Property *prop, const char *value)
+const char *name, const char *value)
{
switch (ret) {
case -EEXIST:
error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
- object_get_typename(obj), prop->name, value);
+ object_get_typename(obj), name, value);
break;
default:
case -EINVAL:
error_setg(errp, QERR_PROPERTY_VALUE_BAD,
- object_get_typename(obj), prop->name, value);
+ object_get_typename(obj), name, value);
break;
case -ENOENT:
error_setg(errp, "Property '%s.%s' can't find value '%s'",
- object_get_typename(obj), prop->name, value);
+ object_get_typename(obj), name, value);
break;
case 0:
break;
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 38fd46b9a9..7a44320d12 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -2390,7 +2390,7 @@ static void set_css_devid(Object *obj, Visitor *v, const
char *name,
num = sscanf(str, "%2x.%1x%n.%4x%n", &cssid, &ssid, &n1, &devid, &n2);
if (num != 3 || (n2 - n1) != 5 || strlen(str) != n2) {
-error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+error_set_from_qdev_prop_erro
[PATCH v4 13/32] qdev: Make qdev_propinfo_get_uint16() static
There are no users of the function outside qdev-properties.c.
Make function static and rename it to get_uint16().
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-prop-internal.h | 2 --
hw/core/qdev-properties.c| 6 +++---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/core/qdev-prop-internal.h b/hw/core/qdev-prop-internal.h
index 9cf5cc1d51..d7b77844fe 100644
--- a/hw/core/qdev-prop-internal.h
+++ b/hw/core/qdev-prop-internal.h
@@ -20,8 +20,6 @@ void qdev_propinfo_set_default_value_int(ObjectProperty *op,
void qdev_propinfo_set_default_value_uint(ObjectProperty *op,
const Property *prop);
-void qdev_propinfo_get_uint16(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp);
void qdev_propinfo_get_int32(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp);
void qdev_propinfo_get_size32(Object *obj, Visitor *v, const char *name,
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 5e010afdb8..765e916c23 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -278,8 +278,8 @@ const PropertyInfo qdev_prop_uint8 = {
/* --- 16bit integer --- */
-void qdev_propinfo_get_uint16(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
+static void get_uint16(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
{
Property *prop = opaque;
uint16_t *ptr = qdev_get_prop_ptr(obj, prop);
@@ -304,7 +304,7 @@ static void set_uint16(Object *obj, Visitor *v, const char
*name,
const PropertyInfo qdev_prop_uint16 = {
.name = "uint16",
-.get = qdev_propinfo_get_uint16,
+.get = get_uint16,
.set = set_uint16,
.set_default_value = qdev_propinfo_set_default_value_uint,
};
--
2.28.0
[PATCH v4 07/32] qdev: Make PropertyInfo.print method get Object* argument
Make the code more generic and not specific to TYPE_DEVICE.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
hw/core/qdev-properties-system.c | 3 ++-
hw/core/qdev-properties.c| 3 +--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index db7ce51dd5..0ea822e6a7 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -32,7 +32,7 @@ struct PropertyInfo {
const char *name;
const char *description;
const QEnumLookup *enum_table;
-int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+int (*print)(Object *obj, Property *prop, char *dest, size_t len);
void (*set_default_value)(ObjectProperty *op, const Property *prop);
void (*create)(ObjectClass *oc, Property *prop);
ObjectPropertyAccessor *get;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 8912fb4e9c..77b31eb9dc 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -801,9 +801,10 @@ invalid:
g_free(str);
}
-static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest,
+static int print_pci_devfn(Object *obj, Property *prop, char *dest,
size_t len)
{
+DeviceState *dev = DEVICE(obj);
int32_t *ptr = qdev_get_prop_ptr(dev, prop);
if (*ptr == -1) {
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index daf844c2d3..b6cf53e929 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -988,13 +988,12 @@ static void qdev_get_legacy_property(Object *obj, Visitor
*v,
const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
char buffer[1024];
char *ptr = buffer;
-prop->info->print(dev, prop, buffer, sizeof(buffer));
+prop->info->print(obj, prop, buffer, sizeof(buffer));
visit_type_str(v, name, &ptr, errp);
}
--
2.28.0
[PATCH v4 12/32] qdev: Make error_set_from_qdev_prop_error() get Object* argument
Make the code more generic and not specific to TYPE_DEVICE.
Reviewed-by: Marc-André Lureau
Reviewed-by: Cornelia Huck #s390 parts
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: qemu-devel@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
hw/core/qdev-properties-system.c | 10 +-
hw/core/qdev-properties.c| 10 +-
hw/s390x/css.c | 2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 7620095fed..530286e869 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -309,7 +309,7 @@ const GlobalProperty *qdev_find_global_prop(Object *obj,
const char *name);
int qdev_prop_check_globals(void);
void qdev_prop_set_globals(DeviceState *dev);
-void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
+void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
Property *prop, const char *value);
/**
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 58bb129bbe..5796ed2619 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -352,7 +352,7 @@ static void set_mac(Object *obj, Visitor *v, const char
*name, void *opaque,
return;
inval:
-error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
g_free(str);
}
@@ -440,7 +440,7 @@ static void set_netdev(Object *obj, Visitor *v, const char
*name,
peers_ptr->queues = queues;
out:
-error_set_from_qdev_prop_error(errp, err, dev, prop, str);
+error_set_from_qdev_prop_error(errp, err, obj, prop, str);
g_free(str);
}
@@ -492,7 +492,7 @@ static void set_audiodev(Object *obj, Visitor *v, const
char* name,
card->state = state;
out:
-error_set_from_qdev_prop_error(errp, err, dev, prop, str);
+error_set_from_qdev_prop_error(errp, err, obj, prop, str);
g_free(str);
}
@@ -790,7 +790,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, const
char *name,
return;
invalid:
-error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
g_free(str);
}
@@ -914,7 +914,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v,
const char *name,
return;
inval:
-error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
g_free(str);
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 41482d83d1..5e010afdb8 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -581,7 +581,7 @@ static void set_uuid(Object *obj, Visitor *v, const char
*name, void *opaque,
if (!strcmp(str, UUID_VALUE_AUTO)) {
qemu_uuid_generate(uuid);
} else if (qemu_uuid_parse(str, uuid) < 0) {
-error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
}
g_free(str);
}
@@ -735,22 +735,22 @@ static Property *qdev_prop_find(DeviceState *dev, const
char *name)
return NULL;
}
-void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
+void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
Property *prop, const char *value)
{
switch (ret) {
case -EEXIST:
error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
- object_get_typename(OBJECT(dev)), prop->name, value);
+ object_get_typename(obj), prop->name, value);
break;
default:
case -EINVAL:
error_setg(errp, QERR_PROPERTY_VALUE_BAD,
- object_get_typename(OBJECT(dev)), prop->name, value);
+ object_get_typename(obj), prop->name, value);
break;
case -ENOENT:
error_setg(errp, "Property '%s.%s' can't find value '%s'",
- object_get_typename(OBJECT(dev)), prop->name, value);
+ object_get_typename(obj), prop->name, value);
break;
case 0:
break;
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 2b8f33fec2..38fd46b9a9 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -2390,7 +2390,7 @@ static void set_css_devid(Object *obj, Visitor *v, const
char *name,
num = sscanf(str, "%2x.%1x%n.%4x%n", &cssid, &ssid, &n1, &devid, &n2);
if (num != 3 || (n2 - n1) != 5 || strlen(str) != n2) {
-error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+error_set_from_q
[PATCH v4 06/32] qdev: Don't use dev->id on set_size32() error message
All other qdev property error messages use "." instead of ".". Change set_size32() for consistency, and to make the code not specific to TYPE_DEVICE. Reviewed-by: Marc-André Lureau Signed-off-by: Eduardo Habkost --- Cc: Paolo Bonzini Cc: "Daniel P. Berrangé" Cc: Eduardo Habkost Cc: qemu-devel@nongnu.org --- hw/core/qdev-properties.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 67ae19df05..daf844c2d3 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -542,7 +542,7 @@ static void set_size32(Object *obj, Visitor *v, const char *name, void *opaque, error_setg(errp, "Property %s.%s doesn't take value %" PRIu64 " (maximum: %u)", - dev->id ? : "", name, value, UINT32_MAX); + object_get_typename(obj), name, value, UINT32_MAX); return; } -- 2.28.0
[PATCH v4 05/32] sparc: Check dev->realized at sparc_set_nwindows()
sparc_set_nwindows() is one of the very few property setters that
don't check dev->realized, and there's no reason for it to be
special. Check dev->realized like the other setters.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Mark Cave-Ayland
Cc: Artyom Tarasenko
Cc: qemu-devel@nongnu.org
---
target/sparc/cpu.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 701e794eac..6a3299041f 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -798,11 +798,17 @@ static void sparc_get_nwindows(Object *obj, Visitor *v,
const char *name,
static void sparc_set_nwindows(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
+DeviceState *dev = DEVICE(obj);
const int64_t min = MIN_NWINDOWS;
const int64_t max = MAX_NWINDOWS;
SPARCCPU *cpu = SPARC_CPU(obj);
int64_t value;
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
if (!visit_type_int(v, name, &value, errp)) {
return;
}
--
2.28.0
[PATCH v4 04/32] qdev: Check dev->realized at set_size()
This setter is one of the very few property setters that don't
check dev->realized, and there's no reason to make size
properties different from the rest. Add the missing check.
Fixes: e8cd45c78f53 ("qdev: Add SIZE type to qdev properties")
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 12a053e732..67ae19df05 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -905,6 +905,11 @@ static void set_size(Object *obj, Visitor *v, const char
*name, void *opaque,
Property *prop = opaque;
uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
visit_type_size(v, name, ptr, errp);
}
--
2.28.0
[PATCH v4 00/32] qdev property code cleanup
This code contains the first 32 patches from the series: Subject: [PATCH v3 00/53] Make qdev static property API usable by any QOM type https://lore.kernel.org/qemu-devel/20201112214350.872250-1-ehabk...@redhat.com I'm submitting this separately so we can merge the qdev-specific cleanup while we discuss our long term plans for QOM properties. Eduardo Habkost (32): cs4231: Get rid of empty property array cpu: Move cpu_common_props to hw/core/cpu.c qdev: Move property code to qdev-properties.[ch] qdev: Check dev->realized at set_size() sparc: Check dev->realized at sparc_set_nwindows() qdev: Don't use dev->id on set_size32() error message qdev: Make PropertyInfo.print method get Object* argument qdev: Make bit_prop_set() get Object* argument qdev: Make qdev_get_prop_ptr() get Object* arg qdev: Make qdev_find_global_prop() get Object* argument qdev: Make check_prop_still_unset() get Object* argument qdev: Make error_set_from_qdev_prop_error() get Object* argument qdev: Make qdev_propinfo_get_uint16() static qdev: Move UUID property to qdev-properties-system.c qdev: Move softmmu properties to qdev-properties-system.h qdev: Reuse DEFINE_PROP in all DEFINE_PROP_* macros sparc: Use DEFINE_PROP for nwindows property qdev: Get just property name at error_set_from_qdev_prop_error() qdev: Avoid using prop->name unnecessarily qdev: Add name parameter to qdev_class_add_property() qdev: Add name argument to PropertyInfo.create method qdev: Wrap getters and setters in separate helpers qdev: Move dev->realized check to qdev_property_set() qdev: Make PropertyInfo.create return ObjectProperty* qdev: Make qdev_class_add_property() more flexible qdev: Separate generic and device-specific property registration qdev: Rename qdev_propinfo_* to field_prop_* qdev: Move qdev_prop_tpm declaration to tpm_prop.h qdev: Rename qdev_prop_* to prop_info_* qdev: Rename qdev_get_prop_ptr() to object_field_prop_ptr() qdev: Avoid unnecessary DeviceState* variable at set_prop_arraylen() tests: Add unit test for qdev array properties audio/audio.h | 1 + hw/core/qdev-prop-internal.h| 76 +++- hw/tpm/tpm_prop.h | 2 + include/hw/block/block.h| 1 + include/hw/core/cpu.h | 1 - include/hw/qdev-core.h | 37 -- include/hw/qdev-properties-system.h | 77 include/hw/qdev-properties.h| 289 ++- include/net/net.h | 1 + backends/tpm/tpm_util.c | 16 +- cpu.c | 15 - hw/acpi/vmgenid.c | 1 + hw/arm/pxa2xx.c | 1 + hw/arm/strongarm.c | 1 + hw/audio/cs4231.c | 5 - hw/block/fdc.c | 1 + hw/block/m25p80.c | 1 + hw/block/nand.c | 1 + hw/block/onenand.c | 1 + hw/block/pflash_cfi01.c | 1 + hw/block/pflash_cfi02.c | 1 + hw/block/vhost-user-blk.c | 1 + hw/block/xen-block.c| 11 +- hw/char/avr_usart.c | 1 + hw/char/bcm2835_aux.c | 1 + hw/char/cadence_uart.c | 1 + hw/char/cmsdk-apb-uart.c| 1 + hw/char/debugcon.c | 1 + hw/char/digic-uart.c| 1 + hw/char/escc.c | 1 + hw/char/etraxfs_ser.c | 1 + hw/char/exynos4210_uart.c | 1 + hw/char/grlib_apbuart.c | 1 + hw/char/ibex_uart.c | 1 + hw/char/imx_serial.c| 1 + hw/char/ipoctal232.c| 1 + hw/char/lm32_juart.c| 1 + hw/char/lm32_uart.c | 1 + hw/char/mcf_uart.c | 1 + hw/char/milkymist-uart.c| 1 + hw/char/nrf51_uart.c| 1 + hw/char/parallel.c | 1 + hw/char/pl011.c | 1 + hw/char/renesas_sci.c | 1 + hw/char/sclpconsole-lm.c| 1 + hw/char/sclpconsole.c | 1 + hw/char/serial-pci-multi.c | 1 + hw/char/serial.c| 1 + hw/char/spapr_vty.c | 1 + hw/char/stm32f2xx_usart.c | 1 + hw/char/terminal3270.c | 1 + hw/char/virtio-console.c| 1 + hw/char/xilinx_uartlite.c | 1 + hw/core/cpu.c | 15 + hw/core/qdev-properties-system.c| 256 ++--- hw/core/qdev-properties.c | 552 +++- hw/core/qdev.c | 120 -- hw/hyperv/vmbus.c | 1 + hw/i386/kvm/i8254.c | 1 + hw/ide/qdev.c | 1 + hw/intc/arm_gicv3_common.c | 2 +- hw/intc/rx_icu.c| 4 +- hw/ipmi/ipmi_bmc_extern.c
[PATCH v4 09/32] qdev: Make qdev_get_prop_ptr() get Object* arg
Make the code more generic and not specific to TYPE_DEVICE.
Reviewed-by: Marc-André Lureau
Reviewed-by: Cornelia Huck #s390 parts
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
- Fix build error with CONFIG_XEN
I took the liberty of keeping the Reviewed-by line from
Marc-André as the build fix is a trivial one line change
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: qemu-devel@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-bl...@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
backends/tpm/tpm_util.c | 8 ++--
hw/block/xen-block.c | 5 +-
hw/core/qdev-properties-system.c | 57 +-
hw/core/qdev-properties.c| 82 +---
hw/s390x/css.c | 5 +-
hw/s390x/s390-pci-bus.c | 4 +-
hw/vfio/pci-quirks.c | 5 +-
8 files changed, 68 insertions(+), 100 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0ea822e6a7..0b92cfc761 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -302,7 +302,7 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name,
const uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
+void *qdev_get_prop_ptr(Object *obj, Property *prop);
void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index e6aeb63587..3973105658 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -35,8 +35,7 @@
static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
-TPMBackend **be = qdev_get_prop_ptr(dev, opaque);
+TPMBackend **be = qdev_get_prop_ptr(obj, opaque);
char *p;
p = g_strdup(*be ? (*be)->id : "");
@@ -49,7 +48,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-TPMBackend *s, **be = qdev_get_prop_ptr(dev, prop);
+TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
char *str;
if (dev->realized) {
@@ -73,9 +72,8 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void release_tpm(Object *obj, const char *name, void *opaque)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-TPMBackend **be = qdev_get_prop_ptr(dev, prop);
+TPMBackend **be = qdev_get_prop_ptr(obj, prop);
if (*be) {
tpm_backend_reset(*be);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 8a7a3f5452..905e4acd97 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -335,9 +335,8 @@ static char *disk_to_vbd_name(unsigned int disk)
static void xen_block_get_vdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(dev, prop);
+XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
char *str;
switch (vdev->type) {
@@ -398,7 +397,7 @@ static void xen_block_set_vdev(Object *obj, Visitor *v,
const char *name,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(dev, prop);
+XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
char *str, *p;
const char *end;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 77b31eb9dc..9ac9b95852 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -59,9 +59,8 @@ static bool check_prop_still_unset(DeviceState *dev, const
char *name,
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(dev, prop);
+void **ptr = qdev_get_prop_ptr(obj, prop);
const char *value;
char *p;
@@ -87,7 +86,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(dev, prop);
+void **ptr = qdev_get_prop_ptr(obj, prop);
char *str;
BlockBackend *blk;
bool blk_created = false;
@@ -185,7 +184,7 @@ static void release_drive(Object *obj, const char *name,
[PATCH v4 03/32] qdev: Move property code to qdev-properties.[ch]
Move everything related to Property and PropertyInfo to
qdev-properties.[ch] to make it easier to refactor that code.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
include/hw/qdev-core.h | 37 ---
include/hw/qdev-properties.h | 38 +++
hw/core/qdev-properties.c| 120 +++
hw/core/qdev.c | 120 ---
softmmu/qdev-monitor.c | 1 +
5 files changed, 159 insertions(+), 157 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 9fbb22a48d..8f91faebc3 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -276,43 +276,6 @@ struct BusState {
ResettableState reset;
};
-/**
- * Property:
- * @set_default: true if the default value should be set from @defval,
- *in which case @info->set_default_value must not be NULL
- *(if false then no default value is set by the property system
- * and the field retains whatever value it was given by instance_init).
- * @defval: default value for the property. This is used only if @set_default
- * is true.
- */
-struct Property {
-const char *name;
-const PropertyInfo *info;
-ptrdiff_toffset;
-uint8_t bitnr;
-bool set_default;
-union {
-int64_t i;
-uint64_t u;
-} defval;
-int arrayoffset;
-const PropertyInfo *arrayinfo;
-int arrayfieldsize;
-const char *link_type;
-};
-
-struct PropertyInfo {
-const char *name;
-const char *description;
-const QEnumLookup *enum_table;
-int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
-void (*set_default_value)(ObjectProperty *op, const Property *prop);
-void (*create)(ObjectClass *oc, Property *prop);
-ObjectPropertyAccessor *get;
-ObjectPropertyAccessor *set;
-ObjectPropertyRelease *release;
-};
-
/**
* GlobalProperty:
* @used: Set to true if property was used when initializing a device.
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 4437450065..db7ce51dd5 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -3,6 +3,44 @@
#include "hw/qdev-core.h"
+/**
+ * Property:
+ * @set_default: true if the default value should be set from @defval,
+ *in which case @info->set_default_value must not be NULL
+ *(if false then no default value is set by the property system
+ * and the field retains whatever value it was given by instance_init).
+ * @defval: default value for the property. This is used only if @set_default
+ * is true.
+ */
+struct Property {
+const char *name;
+const PropertyInfo *info;
+ptrdiff_toffset;
+uint8_t bitnr;
+bool set_default;
+union {
+int64_t i;
+uint64_t u;
+} defval;
+int arrayoffset;
+const PropertyInfo *arrayinfo;
+int arrayfieldsize;
+const char *link_type;
+};
+
+struct PropertyInfo {
+const char *name;
+const char *description;
+const QEnumLookup *enum_table;
+int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+void (*set_default_value)(ObjectProperty *op, const Property *prop);
+void (*create)(ObjectClass *oc, Property *prop);
+ObjectPropertyAccessor *get;
+ObjectPropertyAccessor *set;
+ObjectPropertyRelease *release;
+};
+
+
/*** qdev-properties.c ***/
extern const PropertyInfo qdev_prop_bit;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 509cbf155d..12a053e732 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -929,3 +929,123 @@ const PropertyInfo qdev_prop_link = {
.name = "link",
.create = create_link_property,
};
+
+void qdev_property_add_static(DeviceState *dev, Property *prop)
+{
+Object *obj = OBJECT(dev);
+ObjectProperty *op;
+
+assert(!prop->info->create);
+
+op = object_property_add(obj, prop->name, prop->info->name,
+ prop->info->get, prop->info->set,
+ prop->info->release,
+ prop);
+
+object_property_set_description(obj, prop->name,
+prop->info->description);
+
+if (prop->set_default) {
+prop->info->set_default_value(op, prop);
+if (op->init) {
+op->init(obj, op);
+}
+}
+}
+
+static void qdev_class_add_property(DeviceClass *klass, Property *prop)
+{
+ObjectClass *oc = OBJECT_CLASS(klass);
+
+if (prop->info->create) {
+prop->info->create(oc, prop);
+} else {
+ObjectProperty *op;
+
+op = object_class_property_add(oc,
+ prop->name, prop->info->name,
+ prop->info->get, prop->info->
[PATCH v4 02/32] cpu: Move cpu_common_props to hw/core/cpu.c
There's no reason to keep the property list separate from the CPU
class code. Move the variable to hw/core/cpu.c and make it
static.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Eduardo Habkost
Cc: Marcel Apfelbaum
Cc: qemu-devel@nongnu.org
---
include/hw/core/cpu.h | 1 -
cpu.c | 15 ---
hw/core/cpu.c | 15 +++
3 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 3d92c967ff..8e7552910d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -,7 +,6 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int
asidx);
void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
-extern Property cpu_common_props[];
void cpu_exec_initfn(CPUState *cpu);
void cpu_exec_realizefn(CPUState *cpu, Error **errp);
void cpu_exec_unrealizefn(CPUState *cpu);
diff --git a/cpu.c b/cpu.c
index 0be5dcb6f3..0c485cdf2d 100644
--- a/cpu.c
+++ b/cpu.c
@@ -144,21 +144,6 @@ void cpu_exec_unrealizefn(CPUState *cpu)
#endif
}
-Property cpu_common_props[] = {
-#ifndef CONFIG_USER_ONLY
-/* Create a memory property for softmmu CPU object,
- * so users can wire up its memory. (This can't go in hw/core/cpu.c
- * because that file is compiled only once for both user-mode
- * and system builds.) The default if no link is set up is to use
- * the system address space.
- */
-DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
- MemoryRegion *),
-#endif
-DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false),
-DEFINE_PROP_END_OF_LIST(),
-};
-
void cpu_exec_initfn(CPUState *cpu)
{
cpu->as = NULL;
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 576fa1d7ba..5c89c858aa 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -393,6 +393,21 @@ static vaddr cpu_adjust_watchpoint_address(CPUState *cpu,
vaddr addr, int len)
return addr;
}
+static Property cpu_common_props[] = {
+#ifndef CONFIG_USER_ONLY
+/* Create a memory property for softmmu CPU object,
+ * so users can wire up its memory. (This can't go in hw/core/cpu.c
+ * because that file is compiled only once for both user-mode
+ * and system builds.) The default if no link is set up is to use
+ * the system address space.
+ */
+DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
+ MemoryRegion *),
+#endif
+DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void cpu_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
--
2.28.0
[PATCH v4 08/32] qdev: Make bit_prop_set() get Object* argument
Make the code more generic and not specific to TYPE_DEVICE.
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/core/qdev-properties.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index b6cf53e929..3a4638f4de 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -92,8 +92,9 @@ static uint32_t qdev_get_prop_mask(Property *prop)
return 0x1 << prop->bitnr;
}
-static void bit_prop_set(DeviceState *dev, Property *props, bool val)
+static void bit_prop_set(Object *obj, Property *props, bool val)
{
+DeviceState *dev = DEVICE(obj);
uint32_t *p = qdev_get_prop_ptr(dev, props);
uint32_t mask = qdev_get_prop_mask(props);
if (val) {
@@ -129,7 +130,7 @@ static void prop_set_bit(Object *obj, Visitor *v, const
char *name,
if (!visit_type_bool(v, name, &value, errp)) {
return;
}
-bit_prop_set(dev, prop, value);
+bit_prop_set(obj, prop, value);
}
static void set_default_value_bool(ObjectProperty *op, const Property *prop)
@@ -153,8 +154,9 @@ static uint64_t qdev_get_prop_mask64(Property *prop)
return 0x1ull << prop->bitnr;
}
-static void bit64_prop_set(DeviceState *dev, Property *props, bool val)
+static void bit64_prop_set(Object *obj, Property *props, bool val)
{
+DeviceState *dev = DEVICE(obj);
uint64_t *p = qdev_get_prop_ptr(dev, props);
uint64_t mask = qdev_get_prop_mask64(props);
if (val) {
@@ -190,7 +192,7 @@ static void prop_set_bit64(Object *obj, Visitor *v, const
char *name,
if (!visit_type_bool(v, name, &value, errp)) {
return;
}
-bit64_prop_set(dev, prop, value);
+bit64_prop_set(obj, prop, value);
}
const PropertyInfo qdev_prop_bit64 = {
--
2.28.0
[PATCH v4 01/32] cs4231: Get rid of empty property array
An empty props array is unnecessary, we can just not call
device_class_set_props().
Reviewed-by: Marc-André Lureau
Signed-off-by: Eduardo Habkost
---
Cc: Gerd Hoffmann
Cc: qemu-devel@nongnu.org
---
hw/audio/cs4231.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/hw/audio/cs4231.c b/hw/audio/cs4231.c
index 8e9554ce9b..209c05a0a0 100644
--- a/hw/audio/cs4231.c
+++ b/hw/audio/cs4231.c
@@ -160,17 +160,12 @@ static void cs4231_init(Object *obj)
sysbus_init_irq(dev, &s->irq);
}
-static Property cs4231_properties[] = {
-{.name = NULL},
-};
-
static void cs4231_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
dc->reset = cs_reset;
dc->vmsd = &vmstate_cs4231;
-device_class_set_props(dc, cs4231_properties);
}
static const TypeInfo cs4231_info = {
--
2.28.0
Re: [PATCH 1/1] trace: Send "-d trace:help" output to stdout
Thanks. I gather this is a first step in getting the patch into master? If so, OOC, how many separate "staging" trees are there for the master branch? (is there a list?) On Thu, Dec 10, 2020 at 3:25 AM Stefan Hajnoczi wrote: > On Wed, Nov 25, 2020 at 01:52:45PM -0800, Doug Evans wrote: > > ... for consistency with "-d help". > > > > Signed-off-by: Doug Evans > > --- > > trace/control.c | 12 ++-- > > trace/control.h | 3 ++- > > 2 files changed, 8 insertions(+), 7 deletions(-) > > Thanks, applied to my tracing tree: > https://gitlab.com/stefanha/qemu/commits/tracing > > Stefan >
Re: [PATCH] icount: improve exec nocache usage
On 12/8/20 3:10 AM, Pavel Dovgalyuk wrote:
> cpu-exec tries to execute TB without caching when current
> icount budget is over. But sometimes refilled budget is big
> enough to try executing cached blocks.
> This patch checks that instruction budget is big enough
> for next block execution instead of just running cpu_exec_nocache.
> It halves the number of calls of cpu_exec_nocache function
> during tested OS boot scenario.
>
> Signed-off-by: Pavel Dovgalyuk
> ---
> accel/tcg/cpu-exec.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 58aea605d8..251b340fb9 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -685,7 +685,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
> TranslationBlock *tb,
> insns_left = MIN(0x, cpu->icount_budget);
> cpu_neg(cpu)->icount_decr.u16.low = insns_left;
> cpu->icount_extra = cpu->icount_budget - insns_left;
> -if (!cpu->icount_extra) {
> +if (!cpu->icount_extra && insns_left < tb->icount) {
Reviewed-by: Richard Henderson
I also wonder if we should really be not caching these. Ever since MTTCG, we
have not actually been reusing the memory. We're simply removing the TB from
the hash table. I think we should be remembering these just in case we can in
fact reuse them.
r~
Re: [PATCH 1/2] target/arm: add support for FEAT_DIT, Data Independent Timing
On Fri, 11 Dec 2020 at 19:51, Richard Henderson
wrote:
>
> On 12/11/20 1:33 PM, Rebecca Cran wrote:
> > Is the comment in target/arm/op_helper.c:397 still relevant?
> >
> > uint32_t HELPER(cpsr_read)(CPUARMState *env)
> > {
> > /*
> > * We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr.
> > * This is convenient for populating SPSR_ELx, but must be
> > * hidden from aarch32 mode, where it is not visible.
> > *
> > * TODO: ARMv8.4-DIT -- need to move SS somewhere else.
> > */
> > return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS);
> > }
>
> I forgot about this. So we can't "just" store DIT in uncached_cpsr.
>
> I'll let Peter weigh in, but I think it makes sense to move the SS bit
> somewhere else (e.g. env->pstate) and merge it into SPSR_ELx upon interrupt.
> While what we're doing here is convenient, it's not architectural, and it
> would
> be better to follow GetPSRFromPSTATE pseudocode.
Yes, I think that's the best thing to do. We've been skating
by on CPSR and SPSR being the same thing and it's just
stopped working.
-- PMM
Re: [PATCH v3 1/5] gitlab-ci: Document 'build-tcg-disabled' is a KVM X86 job
On Mon, Dec 7, 2020 at 10:15 AM Philippe Mathieu-Daudé wrote: > > Document what this job cover (build X86 targets with > KVM being the single accelerator available). > > Reviewed-by: Thomas Huth > Signed-off-by: Philippe Mathieu-Daudé > --- > .gitlab-ci.yml | 5 + > 1 file changed, 5 insertions(+) Reviewed-by: WIllian Rampazzo
[PATCH v3] colo: Use class properties
Instance properties make introspection hard and are not shown by
"-object ...,help". Convert them to class properties.
Signed-off-by: Eduardo Habkost
---
Changes v2 -> v3:
* Type of "compare_timeout" was changed to uint64
* Indent fix at "compare_thread" registration statement
Notes v2:
This was originally submitted as part of the series:
Subject: [PATCH 00/12] qom: Make all -object types use only class properties
Message-Id: <20201009160122.1662082-1-ehabk...@redhat.com>
https://lore.kernel.org/qemu-devel/20201009160122.1662082-1-ehabk...@redhat.com
---
Cc: Zhang Chen
Cc: Li Zhijian
Cc: Jason Wang
Cc: qemu-devel@nongnu.org
---
net/colo-compare.c | 57 +++---
1 file changed, 29 insertions(+), 28 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 337025b44f..aef8271341 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1377,6 +1377,35 @@ static void colo_compare_class_init(ObjectClass *oc,
void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+object_class_property_add_str(oc, "primary_in",
+ compare_get_pri_indev,
compare_set_pri_indev);
+object_class_property_add_str(oc, "secondary_in",
+ compare_get_sec_indev,
compare_set_sec_indev);
+object_class_property_add_str(oc, "outdev",
+ compare_get_outdev, compare_set_outdev);
+object_class_property_add_link(oc, "iothread", TYPE_IOTHREAD,
+ offsetof(CompareState, iothread),
+ object_property_allow_set_link,
+ OBJ_PROP_LINK_STRONG);
+/* This parameter just for Xen COLO */
+object_class_property_add_str(oc, "notify_dev",
+ compare_get_notify_dev,
compare_set_notify_dev);
+
+object_class_property_add(oc, "compare_timeout", "uint64",
+ compare_get_timeout,
+ compare_set_timeout, NULL, NULL);
+
+object_class_property_add(oc, "expired_scan_cycle", "uint32",
+ compare_get_expired_scan_cycle,
+ compare_set_expired_scan_cycle, NULL, NULL);
+
+object_class_property_add(oc, "max_queue_size", "uint32",
+ get_max_queue_size,
+ set_max_queue_size, NULL, NULL);
+
+object_class_property_add_bool(oc, "vnet_hdr_support",
compare_get_vnet_hdr,
+ compare_set_vnet_hdr);
+
ucc->complete = colo_compare_complete;
}
@@ -1384,35 +1413,7 @@ static void colo_compare_init(Object *obj)
{
CompareState *s = COLO_COMPARE(obj);
-object_property_add_str(obj, "primary_in",
-compare_get_pri_indev, compare_set_pri_indev);
-object_property_add_str(obj, "secondary_in",
-compare_get_sec_indev, compare_set_sec_indev);
-object_property_add_str(obj, "outdev",
-compare_get_outdev, compare_set_outdev);
-object_property_add_link(obj, "iothread", TYPE_IOTHREAD,
-(Object **)&s->iothread,
-object_property_allow_set_link,
-OBJ_PROP_LINK_STRONG);
-/* This parameter just for Xen COLO */
-object_property_add_str(obj, "notify_dev",
-compare_get_notify_dev, compare_set_notify_dev);
-
-object_property_add(obj, "compare_timeout", "uint64",
-compare_get_timeout,
-compare_set_timeout, NULL, NULL);
-
-object_property_add(obj, "expired_scan_cycle", "uint32",
-compare_get_expired_scan_cycle,
-compare_set_expired_scan_cycle, NULL, NULL);
-
-object_property_add(obj, "max_queue_size", "uint32",
-get_max_queue_size,
-set_max_queue_size, NULL, NULL);
-
s->vnet_hdr = false;
-object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
- compare_set_vnet_hdr);
}
static void colo_compare_finalize(Object *obj)
--
2.28.0
Re: [PATCH v2 09/12] colo: Use class properties
On Fri, Nov 13, 2020 at 03:04:55AM +, Zhang, Chen wrote: > Looks good for me, but Qemu still have lots of parts use > object_property_add, do you have plan to change it? Thanks! The plan is to minimize usage of object_property_add(), but that's a huge task. We're doing it gradually when it is more useful (like on user-visible devices or backend objects). -- Eduardo
Re: [PATCH v2 00/12] qom: Convert some properties to class properties
On Wed, Nov 11, 2020 at 01:38:11PM -0500, Eduardo Habkost wrote: > Class properties make QOM introspection simpler and easier, as it > doesn't require an object to be instantiated. This series > converts a few existing object_property_add*() calls to register > class properties instead. > > Changes v1 -> v2: > * Bug fix at "i386: Register feature bit properties as class properties" > * Included patches that were originally submnitted as part of > "qom: Make all -object types use only class properties" > * All other patches are unchanged from v1 I'm queueing this series, excluding patch 12/12. -- Eduardo
Re: [PATCH 1/1] scsi: fix device removal race vs IO restart callback on resume
On 10/12/20 13:59, Maxim Levitsky wrote:
There is (mostly theoretical) race between removal of a scsi device and
scsi_dma_restart_bh.
It used to be easier to hit this race prior to my / Paulo's patch series
that added rcu to scsi bus device handling code, but IMHO this race
should still be possible to hit, at least in theory.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1854811
Fix it anyway with a patch that was proposed by Paulo in the above bugzilla.
Suggested-by: Paolo Bonzini
Signed-off-by: Maxim Levitsky
---
hw/scsi/scsi-bus.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index b901e701f0..edb5c3492a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -170,6 +170,8 @@ static void scsi_dma_restart_bh(void *opaque)
scsi_req_unref(req);
}
aio_context_release(blk_get_aio_context(s->conf.blk));
+/* Drop the reference that was acquired in scsi_dma_restart_cb */
+object_unref(OBJECT(s));
}
void scsi_req_retry(SCSIRequest *req)
@@ -188,6 +190,8 @@ static void scsi_dma_restart_cb(void *opaque, int running,
RunState state)
}
if (!s->bh) {
AioContext *ctx = blk_get_aio_context(s->conf.blk);
+/* The reference is dropped in scsi_dma_restart_bh.*/
+object_ref(OBJECT(s));
s->bh = aio_bh_new(ctx, scsi_dma_restart_bh, s);
qemu_bh_schedule(s->bh);
}
Queued, thanks.
Paolo
Re: [PATCH 0/7] Additional NPCM7xx devices and IPMI BMC emulation support
Tl,dr: We'll remove the IPMI changes from the current patch set and refactor them in a separate patch set. Thank you for your review! On high level, we are trying to emulate the BMC side of the IPMI protocol. So we cannot directly use the existing IPMI code. However, they do have a lot in duplication as you pointed out. So we'll refactor the existing IPMI code and update in a way that we only add the required functionality. As for the KCS module, the BMC side of the protocol is the opposite direction of the existing ipmi_kcs.c code which is on the host/CPU side. For example, in READ_STATE the CPU would read data while the BMC would write data. So we can't directly use the same implementation. (They're different files in Linux either.) However, we can refactor it to re-use some of the common definitions. We would like to remove the IPMI and KCS stuff from the current patch set. We'll send the refactored code in a separate patch set after addressing your concerns. Thanks again for the review! On Thu, Dec 10, 2020 at 7:04 PM Corey Minyard wrote: > On Thu, Dec 10, 2020 at 05:51:49PM -0800, Hao Wu wrote: > > This patch series include a few more NPCM7XX devices including > > > > - Analog Digital Converter (ADC) > > - Pulse Width Modulation (PWM) > > - Keyboard Style Controller (KSC) > > > > To utilize these modules we also add two extra functionalities: > > > > 1. We modified the CLK module to generate clock values using qdev_clock. > >These clocks are used to determine various clocks in NPCM7XX devices. > > 2. We added support for emulating IPMI responder devices in BMC machines, > >similar to the existing IPMI device support for CPU emulation. This > allows > >a qemu instance running BMC firmware to serve as an external BMC for > a qemu > >instance running server software. It utilizes the KCS module we > implemented. > > Looking at the IPMI changes, why didn't you just re-use the existing > IPMI infrastructure? ipmi_host.[ch] is basically a subset of ipmi.[ch], > and the ipmi_host_extern looks like a copy of of ipmi_bmc_extern with > some names changed. That kind of code duplication is not acceptable. > Plus you copied my code and removed my copyrights, which is really > not acceptable and illegal. > > I'm not exactly sure why you needed you own KCS interface, either. It > looks like the interface is somewhat different in some ways, but > integrating it into the current KCS code is probably a better choice if > that can be done. > > -corey > > > > > Hao Wu (7): > > hw/misc: Add clock converter in NPCM7XX CLK module > > hw/timer: Refactor NPCM7XX Timer to use CLK clock > > hw/adc: Add an ADC module for NPCM7XX > > hw/misc: Add a PWM module for NPCM7XX > > hw/ipmi: Add an IPMI host interface > > hw/ipmi: Add a KCS Module for NPCM7XX > > hw/ipmi: Add an IPMI external host device > > > > default-configs/devices/arm-softmmu.mak | 2 + > > docs/system/arm/nuvoton.rst | 6 +- > > hw/adc/meson.build | 1 + > > hw/adc/npcm7xx_adc.c| 318 ++ > > hw/arm/npcm7xx.c| 65 +- > > hw/ipmi/Kconfig | 5 + > > hw/ipmi/ipmi_host.c | 40 ++ > > hw/ipmi/ipmi_host_extern.c | 435 + > > hw/ipmi/meson.build | 3 + > > hw/ipmi/npcm7xx_kcs.c | 570 + > > hw/misc/meson.build | 1 + > > hw/misc/npcm7xx_clk.c | 795 +++- > > hw/misc/npcm7xx_pwm.c | 535 > > hw/timer/npcm7xx_timer.c| 25 +- > > include/hw/adc/npcm7xx_adc.h| 72 +++ > > include/hw/arm/npcm7xx.h| 6 + > > include/hw/ipmi/ipmi_host.h | 56 ++ > > include/hw/ipmi/ipmi_responder.h| 66 ++ > > include/hw/ipmi/npcm7xx_kcs.h | 105 > > include/hw/misc/npcm7xx_clk.h | 146 - > > include/hw/misc/npcm7xx_pwm.h | 106 > > include/hw/timer/npcm7xx_timer.h| 1 + > > tests/qtest/meson.build | 4 +- > > tests/qtest/npcm7xx_adc-test.c | 400 > > tests/qtest/npcm7xx_pwm-test.c | 490 +++ > > 25 files changed, 4221 insertions(+), 32 deletions(-) > > create mode 100644 hw/adc/npcm7xx_adc.c > > create mode 100644 hw/ipmi/ipmi_host.c > > create mode 100644 hw/ipmi/ipmi_host_extern.c > > create mode 100644 hw/ipmi/npcm7xx_kcs.c > > create mode 100644 hw/misc/npcm7xx_pwm.c > > create mode 100644 include/hw/adc/npcm7xx_adc.h > > create mode 100644 include/hw/ipmi/ipmi_host.h > > create mode 100644 include/hw/ipmi/ipmi_responder.h > > create mode 100644 include/hw/ipmi/npcm7xx_kcs.h > > create mode 100644 include/hw/misc/npcm7xx_pwm.h > > create mode 100644 tests/qtest/npcm7xx_adc-test.c > > create mode 100644 tests/qtest/npcm7xx_pwm-test.c > > > >
Re: [PATCH] icount: improve exec nocache usage
On 08/12/20 10:10, Pavel Dovgalyuk wrote:
cpu-exec tries to execute TB without caching when current
icount budget is over. But sometimes refilled budget is big
enough to try executing cached blocks.
This patch checks that instruction budget is big enough
for next block execution instead of just running cpu_exec_nocache.
It halves the number of calls of cpu_exec_nocache function
during tested OS boot scenario.
Signed-off-by: Pavel Dovgalyuk
---
accel/tcg/cpu-exec.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 58aea605d8..251b340fb9 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -685,7 +685,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu,
TranslationBlock *tb,
insns_left = MIN(0x, cpu->icount_budget);
cpu_neg(cpu)->icount_decr.u16.low = insns_left;
cpu->icount_extra = cpu->icount_budget - insns_left;
-if (!cpu->icount_extra) {
+if (!cpu->icount_extra && insns_left < tb->icount) {
/* Execute any remaining instructions, then let the main loop
* handle the next event.
*/
Queued, thanks.
Paolo
Re: [PATCH 3/3] tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon device
On 12/11/20 2:31 PM, Thomas Huth wrote:
Inflate the balloon and check whether the size of the memory changes.
Yeah, because a true balloon should inflate :D
Signed-off-by: Thomas Huth
---
tests/acceptance/machine_s390_ccw_virtio.py | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
Reviewed-by: Wainer dos Santos Moschetta
diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
b/tests/acceptance/machine_s390_ccw_virtio.py
index 7d0a78139b..81f6c066c7 100644
--- a/tests/acceptance/machine_s390_ccw_virtio.py
+++ b/tests/acceptance/machine_s390_ccw_virtio.py
@@ -70,7 +70,8 @@ class S390CCWVirtioMachine(Test):
'-device', 'zpci,uid=5,target=zzz',
'-device', 'virtio-net-pci,id=zzz',
'-device', 'zpci,uid=0xa,fid=12,target=serial',
- '-device', 'virtio-serial-pci,id=serial')
+ '-device', 'virtio-serial-pci,id=serial',
+ '-device', 'virtio-balloon-ccw')
self.vm.launch()
shell_ready = "sh: can't access tty; job control turned off"
@@ -140,3 +141,12 @@ class S390CCWVirtioMachine(Test):
exec_command_and_wait_for_pattern(self,
'ls /sys/bus/ccw/devices/0.0.4711',
'No such file or directory')
+# test the virtio-balloon device
+exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
+ 'MemTotal: 115640 kB')
+self.vm.command('human-monitor-command', command_line='balloon 96')
+exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
+ 'MemTotal: 82872 kB')
+self.vm.command('human-monitor-command', command_line='balloon 128')
+exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
+ 'MemTotal: 115640 kB')
Re: [PATCH v11 05/25] i386: move whpx accel files into whpx/
On Fri, Dec 11, 2020 at 09:31:23AM +0100, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana
> Reviewed-by: Alex Bennée
> ---
> target/i386/{ => whpx}/whp-dispatch.h | 0
> target/i386/{ => whpx}/whpx-cpus.h| 0
> target/i386/{ => whpx}/whpx-all.c | 0
> target/i386/{ => whpx}/whpx-cpus.c| 0
> MAINTAINERS | 5 +
> target/i386/meson.build | 5 +
> target/i386/whpx/meson.build | 4
> 7 files changed, 6 insertions(+), 8 deletions(-)
> rename target/i386/{ => whpx}/whp-dispatch.h (100%)
> rename target/i386/{ => whpx}/whpx-cpus.h (100%)
> rename target/i386/{ => whpx}/whpx-all.c (100%)
> rename target/i386/{ => whpx}/whpx-cpus.c (100%)
> create mode 100644 target/i386/whpx/meson.build
>
Unfortunately this conflicts with commit faf20793b5af ("WHPX:
support for the kernel-irqchip on/off") and needs to be redone.
--
Eduardo
Re: [PATCH 2/3] tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via /dev/hwrng
Hi,
On 12/11/20 2:31 PM, Thomas Huth wrote:
/dev/hwrng is only functional if virtio-rng is working right, so let's
add a sanity check for this device node.
Good idea.
Signed-off-by: Thomas Huth
---
tests/acceptance/machine_s390_ccw_virtio.py | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
b/tests/acceptance/machine_s390_ccw_virtio.py
index 733a7ca24a..7d0a78139b 100644
--- a/tests/acceptance/machine_s390_ccw_virtio.py
+++ b/tests/acceptance/machine_s390_ccw_virtio.py
@@ -64,9 +64,9 @@ class S390CCWVirtioMachine(Test):
'-append', kernel_command_line,
'-device', 'virtio-net-ccw,devno=fe.1.',
'-device',
- 'virtio-rng-ccw,devno=fe.2.,max_revision=0',
+
'virtio-rng-ccw,devno=fe.2.,max_revision=0,id=rn1',
'-device',
- 'virtio-rng-ccw,devno=fe.3.1234,max_revision=2',
+
'virtio-rng-ccw,devno=fe.3.1234,max_revision=2,id=rn2',
'-device', 'zpci,uid=5,target=zzz',
'-device', 'virtio-net-pci,id=zzz',
'-device', 'zpci,uid=0xa,fid=12,target=serial',
@@ -96,6 +96,19 @@ class S390CCWVirtioMachine(Test):
exec_command_and_wait_for_pattern(self,
'cat /sys/bus/ccw/devices/0.3.1234/virtio?/features',
virtio_rng_features)
+# check that /dev/hwrng works - and that it's gone after ejecting
+exec_command_and_wait_for_pattern(self,
+'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
+'10+0 records out')
+self.clear_guests_dmesg()
+self.vm.command('device_del', id='rn1')
+self.wait_for_crw_reports()
+self.clear_guests_dmesg()
+self.vm.command('device_del', id='rn2')
+self.wait_for_crw_reports()
+exec_command_and_wait_for_pattern(self,
+'dd if=/dev/hwrng of=/tmp/out.dat bs=1k count=10',
+'dd: /dev/hwrng: No such device')
Maybe the expected pattern is too fragile. On my Fedora 33 system, 'dd'
will print a different message.
What if it checks for the presence of the device file, e.g:
... self, 'test -c /dev/hwrng; echo $?', '1')
- Wainer
# verify that we indeed have virtio-net devices (without having the
# virtio-net driver handy)
exec_command_and_wait_for_pattern(self,
Re: Some performance numbers for virtiofs, DAX and virtio-9p
* Vivek Goyal (vgo...@redhat.com) wrote: > On Fri, Dec 11, 2020 at 02:25:17PM -0500, Vivek Goyal wrote: > > On Fri, Dec 11, 2020 at 06:29:56PM +, Dr. David Alan Gilbert wrote: > > > > [..] > > > > > > > > > > Could we measure at what point does a large window size actually make > > > > > performance worse? > > > > > > > > Will do. Will run tests with varying window sizes (small to large) > > > > and see how does it impact performance for same workload with > > > > same guest memory. > > > > > > I wonder how realistic it is though; it makes some sense if you have a > > > scenario like a fairly small root filesystem - something tractable; but > > > if you have a large FS you're not realistically going to be able to set > > > the cache size to match it - that's why it's a cache! > > > > I think its more about active dataset size and not necessarily total > > FS size. FS might be big but if application is not accessing all of > > the in reasonabl small time window, then it does not matter. > > > > What worries me most is that cost of reclaim of a dax range seems > > too high (or keeps the process blocked for long enogh), that it > > kills the performance. I will need to revisit the reclaim path > > and see if I can optimize something. > > I see that while reclaiming a range, we are sending a remvemapping > command to virtiofsd. We are holding locks so that no new mappings > can be added to that inode. Are you doing this when mapping something new in as well? If you're replacing an existing mapping then there's no downside to just skipping the remove. > We had decided that removemapping is not necessary. It helps in > the sense that if guest is not using a mapping, qemu will unmap > it too. > > If we stop sending remove mapping, then it might improve reclaim > performance significantly. With the downside that host will > have something mapped (despite the fact that guest is not using > it anymore). And these will be cleaned up only when guest shuts > down. There's probably some scope for improving this on the QEMU side as well; although there's always going to be latency of going through the path to do the request. Dave > > Vivek -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH 0/3] tests/acceptance: Test virtio-rng and -balloon on s390x
On 12/11/20 2:31 PM, Thomas Huth wrote: Add two more simple tests to check that virtio-rng and virtio-balloon are at least (very) basically working on s390x. Based-on: 20201204121450.120730-1-coh...@redhat.com Thomas Huth (3): tests/acceptance: Extract the code to clear dmesg and wait for CRW reports tests/acceptance/machine_s390_ccw_virtio: Test virtio-rng via /dev/hwrng tests/acceptance/machine_s390_ccw_virtio: Test the virtio-balloon device tests/acceptance/machine_s390_ccw_virtio.py | 59 +++-- 1 file changed, 43 insertions(+), 16 deletions(-) One observation, test_s390x_devices tends to get longer and difficult to debug in case of problems. If a test covers one specific device type, It will improve readability, flexibility, and debugging. In case you don't want to spend time breaking this into multiple tests, I'll be glad to do that after the whole series is merged. As far as code concerned, Reviewed-by: Willian Rampazzo Tested-by: Willian Rampazzo Fetching asset from tests/acceptance/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices Fetching asset from tests/acceptance/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices JOB ID : 8ba8e572f2582f9a48f2542423342e51e257db97 JOB LOG: /home/linux1/src/qemu.dev/build/tests/results/job-2020-12-11T15.01-8ba8e57/job.log (1/1) tests/acceptance/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices: PASS (7.89 s) RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 JOB TIME : 8.03 s
Re: [PATCH 11/20] Revert "qobject: let object_property_get_str() use new API"
On Fri, Dec 11, 2020 at 06:11:43PM +0100, Markus Armbruster wrote: > Commit aafb21a0b9 "qobject: let object_property_get_str() use new API" > isn't much of a simplification. Not worth having > object_property_get_str() differ from the other > object_property_get_FOO(). Revert. > > This reverts commit aafb21a0b9cea5fa0fe52e68111bb6bd13837a02. > > Cc: Paolo Bonzini > Cc: Daniel P. Berrangé > Cc: Eduardo Habkost > Signed-off-by: Markus Armbruster Reviewed-by: Eduardo Habkost -- Eduardo
Re: [PATCH v3 06/13] virtiofsd: replace _Static_assert with QEMU_BUILD_BUG_ON
* marcandre.lur...@redhat.com (marcandre.lur...@redhat.com) wrote:
> From: Marc-André Lureau
>
> This allows to get rid of a check for older GCC version (which was a bit
> bogus too since it was falling back on c++ version..)
>
> Signed-off-by: Marc-André Lureau
Yes I think that's OK; this is an imported file, but we've already
mangled it into QEMU's style and added includes etc.
Reviewed-by: Dr. David Alan Gilbert
> ---
> tools/virtiofsd/fuse_common.h | 11 +--
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
> index 5aee5193eb..a2484060b6 100644
> --- a/tools/virtiofsd/fuse_common.h
> +++ b/tools/virtiofsd/fuse_common.h
> @@ -809,15 +809,6 @@ void fuse_remove_signal_handlers(struct fuse_session
> *se);
> *
> * On 32bit systems please add -D_FILE_OFFSET_BITS=64 to your compile flags!
> */
> -
> -#if defined(__GNUC__) && \
> -(__GNUC__ > 4 || __GNUC__ == 4 && __GNUC_MINOR__ >= 6) && \
> -!defined __cplusplus
> -_Static_assert(sizeof(off_t) == 8, "fuse: off_t must be 64bit");
> -#else
> -struct _fuse_off_t_must_be_64bit_dummy_struct {
> -unsigned _fuse_off_t_must_be_64bit:((sizeof(off_t) == 8) ? 1 : -1);
> -};
> -#endif
> +QEMU_BUILD_BUG_ON(sizeof(off_t) != 8);
>
> #endif /* FUSE_COMMON_H_ */
> --
> 2.29.0
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH 1/3] tests/acceptance: Extract the code to clear dmesg and wait for CRW reports
Hi,
On 12/11/20 2:31 PM, Thomas Huth wrote:
We will use this in more spots soon, so it's easier to put this into
a separate function.
Signed-off-by: Thomas Huth
---
tests/acceptance/machine_s390_ccw_virtio.py | 30 -
1 file changed, 17 insertions(+), 13 deletions(-)
Reviewed-by: Wainer dos Santos Moschetta
diff --git a/tests/acceptance/machine_s390_ccw_virtio.py
b/tests/acceptance/machine_s390_ccw_virtio.py
index 864ef4ee6e..733a7ca24a 100644
--- a/tests/acceptance/machine_s390_ccw_virtio.py
+++ b/tests/acceptance/machine_s390_ccw_virtio.py
@@ -17,12 +17,24 @@ from avocado_qemu import wait_for_console_pattern
class S390CCWVirtioMachine(Test):
KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
+timeout = 120
+
def wait_for_console_pattern(self, success_message, vm=None):
wait_for_console_pattern(self, success_message,
failure_message='Kernel panic - not syncing',
vm=vm)
-timeout = 120
+def wait_for_crw_reports(self):
+exec_command_and_wait_for_pattern(self,
+'while ! (dmesg -c | grep CRW) ; do sleep 1 ; done',
+'CRW reports')
+
+dmesg_clear_count = 1
+def clear_guests_dmesg(self):
+exec_command_and_wait_for_pattern(self, 'dmesg -c > /dev/null; '
+'echo dm_clear\ ' + str(self.dmesg_clear_count),
+'dm_clear ' + str(self.dmesg_clear_count))
+self.dmesg_clear_count += 1
def test_s390x_devices(self):
@@ -100,26 +112,18 @@ class S390CCWVirtioMachine(Test):
'cat /sys/bus/pci/devices/000a\:00\:00.0/function_id',
'0x000c')
# add another device
-exec_command_and_wait_for_pattern(self,
-'dmesg -c > /dev/null; echo dm_clear\ 1',
-'dm_clear 1')
+self.clear_guests_dmesg()
self.vm.command('device_add', driver='virtio-net-ccw',
devno='fe.0.4711', id='net_4711')
-exec_command_and_wait_for_pattern(self,
-'while ! (dmesg -c | grep CRW) ; do sleep 1 ; done',
-'CRW reports')
+self.wait_for_crw_reports()
exec_command_and_wait_for_pattern(self, 'ls /sys/bus/ccw/devices/',
'0.0.4711')
# and detach it again
-exec_command_and_wait_for_pattern(self,
-'dmesg -c > /dev/null; echo dm_clear\ 2',
-'dm_clear 2')
+self.clear_guests_dmesg()
self.vm.command('device_del', id='net_4711')
self.vm.event_wait(name='DEVICE_DELETED',
match={'data': {'device': 'net_4711'}})
-exec_command_and_wait_for_pattern(self,
-'while ! (dmesg -c | grep CRW) ; do sleep 1 ; done',
-'CRW reports')
+self.wait_for_crw_reports()
exec_command_and_wait_for_pattern(self,
'ls /sys/bus/ccw/devices/0.0.4711',
'No such file or directory')
Re: [PATCH v11 18/25] cpu: Move synchronize_from_tb() to tcg_ops
On Fri, Dec 11, 2020 at 07:51:54PM +0100, Claudio Fontana wrote: > On 12/11/20 7:26 PM, Philippe Mathieu-Daudé wrote: > > On 12/11/20 7:22 PM, Richard Henderson wrote: > >> On 12/11/20 12:15 PM, Claudio Fontana wrote: > >>> Should I return this file to the original state (without the extra > >>> #includes that pretend it to be a standalone header file, > >>> and call it > >>> > >>> tcg-cpu-ops.h.inc > >>> > >>> ? > >> > >> If this header can work with qemu/typedefs.h, then no, because the > >> circularity > >> has been resolved. Otherwise, yes. > > > > My editor got confused with TranslationBlock, which is why I asked > > to include its declaration. > > > > Easier to forward-declare TranslationBlock in qemu/typedefs.h? > > > > Regards, > > > > Phil. > > > > Hello Philippe, > > ok you propose to move the existing fwd declaration of TranslationBlock from > cpu.h to qemu/typedefs.h . It seems simpler to just add a typedef struct TranslationBlock TranslationBlock; line to tcg-cpu-ops.h. Or, an even simpler solution: just use `struct TranslationBlock` instead of `TranslationBlock` in the declarations being moved to tcg-cpu-ops.h. We don't need to move declarations to typedefs.h anymore, because now the compilers we support don't warn about typedef redefinitions: https://lore.kernel.org/qemu-devel/20200914134636.gz1618...@habkost.net/ > > And what about #include "exec/memattrs.h"? > > I assume you propose to put struct MemTxAttrs there as a fwd declaration too, This can't be done, because MemTxAttrs can't be an incomplete type in the code you are moving (the methods get a MemTxAttrs value, not a pointer). > > and this concludes our experiment here? > > Thanks, > > Claudio > -- Eduardo
Re: Some performance numbers for virtiofs, DAX and virtio-9p
On Fri, Dec 11, 2020 at 02:25:17PM -0500, Vivek Goyal wrote: > On Fri, Dec 11, 2020 at 06:29:56PM +, Dr. David Alan Gilbert wrote: > > [..] > > > > > > > > Could we measure at what point does a large window size actually make > > > > performance worse? > > > > > > Will do. Will run tests with varying window sizes (small to large) > > > and see how does it impact performance for same workload with > > > same guest memory. > > > > I wonder how realistic it is though; it makes some sense if you have a > > scenario like a fairly small root filesystem - something tractable; but > > if you have a large FS you're not realistically going to be able to set > > the cache size to match it - that's why it's a cache! > > I think its more about active dataset size and not necessarily total > FS size. FS might be big but if application is not accessing all of > the in reasonabl small time window, then it does not matter. > > What worries me most is that cost of reclaim of a dax range seems > too high (or keeps the process blocked for long enogh), that it > kills the performance. I will need to revisit the reclaim path > and see if I can optimize something. I see that while reclaiming a range, we are sending a remvemapping command to virtiofsd. We are holding locks so that no new mappings can be added to that inode. We had decided that removemapping is not necessary. It helps in the sense that if guest is not using a mapping, qemu will unmap it too. If we stop sending remove mapping, then it might improve reclaim performance significantly. With the downside that host will have something mapped (despite the fact that guest is not using it anymore). And these will be cleaned up only when guest shuts down. Vivek
Re: [PATCH 1/2] target/arm: add support for FEAT_DIT, Data Independent Timing
On 12/11/20 7:08 AM, Richard Henderson wrote:
Alternately, or additionally, TCG is outside of the security domain (only
hardware accelerators like KVM are inside), and so we may implement this as a
NOP.
Thanks for the feedback, I'll send a v2 patch next week.
Is the comment in target/arm/op_helper.c:397 still relevant?
uint32_t HELPER(cpsr_read)(CPUARMState *env)
{
/*
* We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr.
* This is convenient for populating SPSR_ELx, but must be
* hidden from aarch32 mode, where it is not visible.
*
* TODO: ARMv8.4-DIT -- need to move SS somewhere else.
*/
return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS);
}
--
Rebecca Cran
Re: [PATCH 3/3] virtiofsd: Check file type in lo_flush()
* Vivek Goyal (vgo...@redhat.com) wrote:
> On Thu, Dec 10, 2020 at 08:14:31PM +, Dr. David Alan Gilbert wrote:
> > * Vivek Goyal (vgo...@redhat.com) wrote:
> > > On Thu, Dec 10, 2020 at 08:03:03PM +, Dr. David Alan Gilbert wrote:
> > > > * Vivek Goyal (vgo...@redhat.com) wrote:
> > > > > Currently lo_flush() is written in such a way that it expects to
> > > > > receive
> > > > > a FLUSH requests on a regular file (and not directories). For example,
> > > > > we call lo_fi_fd() which searches lo->fd_map. If we open directories
> > > > > using opendir(), we keep don't keep track of these in lo->fd_map
> > > > > instead
> > > > > we keep them in lo->dir_map. So we expect lo_flush() to be called on
> > > > > regular files only.
> > > > >
> > > > > Even linux fuse client calls FLUSH only for regular files and not
> > > > > directories. So put a check for filetype and return EBADF if
> > > > > lo_flush() is called on a non-regular file.
> > > > >
> > > > > Reported-by: Laszlo Ersek
> > > > > Signed-off-by: Vivek Goyal
> > > > > ---
> > > > > tools/virtiofsd/passthrough_ll.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tools/virtiofsd/passthrough_ll.c
> > > > > b/tools/virtiofsd/passthrough_ll.c
> > > > > index 8ba79f503a..48a109d3f6 100644
> > > > > --- a/tools/virtiofsd/passthrough_ll.c
> > > > > +++ b/tools/virtiofsd/passthrough_ll.c
> > > > > @@ -1968,7 +1968,7 @@ static void lo_flush(fuse_req_t req, fuse_ino_t
> > > > > ino, struct fuse_file_info *fi)
> > > > > struct lo_data *lo = lo_data(req);
> > > > >
> > > > > inode = lo_inode(req, ino);
> > > > > -if (!inode) {
> > > > > +if (!inode || !S_ISREG(inode->filetype)) {
> > > > > fuse_reply_err(req, EBADF);
> > > >
> > > > Does that need a lo_inode_put(lo, &inode) in the new case?
> > >
> > > Good catch. Yes if inode is valid but file type is not regular, we need
> > > to put inode reference.
> > >
> > > Do you want me to post a new patch or you will like to take care of
> > > it.
> >
> > OK, so if we make this :
> >
> > if (!inode) {
> > fuse_reply_err(req, EBADF);
> > return;
> > }
> >
> > if (!S_ISREG(inode->filetype)) {
> > lo_inode_put(lo_data(req), &inode);
> > fuse_reply_err(req, EBADF);
> > return;
> > }
> >
> > (Untested)
>
> Hi Dave,
>
> Above looks good. For your convenience, I updated the patch and also
> tested it by running blogbench and things look fine.
>
> Vivek
>
> Subject: virtiofsd: Check file type in lo_flush()
>
> Currently lo_flush() is written in such a way that it expects to receive
> a FLUSH requests on a regular file (and not directories). For example,
> we call lo_fi_fd() which searches lo->fd_map. If we open directories
> using opendir(), we keep don't keep track of these in lo->fd_map instead
> we keep them in lo->dir_map. So we expect lo_flush() to be called on
> regular files only.
>
> Even linux fuse client calls FLUSH only for regular files and not
> directories. So put a check for filetype and return EBADF if
> lo_flush() is called on a non-regular file.
>
> Reported-by: Laszlo Ersek
> Signed-off-by: Vivek Goyal
OK< thanks
Reviewed-by: Dr. David Alan Gilbert
and queued.
(I've dropped the Tested-by since it's had a bit of forceful rework).
> ---
> tools/virtiofsd/passthrough_ll.c |6 ++
> 1 file changed, 6 insertions(+)
>
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2020-12-11
> 09:00:28.787669761 -0500
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c2020-12-11
> 09:03:38.239496505 -0500
> @@ -1973,6 +1973,12 @@ static void lo_flush(fuse_req_t req, fus
> return;
> }
>
> +if (!S_ISREG(inode->filetype)) {
> +lo_inode_put(lo, &inode);
> +fuse_reply_err(req, EBADF);
> +return;
> +}
> +
> /* An fd is going away. Cleanup associated posix locks */
> if (lo->posix_lock) {
> pthread_mutex_lock(&inode->plock_mutex);
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH 1/2] target/arm: add support for FEAT_DIT, Data Independent Timing
On 12/11/20 1:33 PM, Rebecca Cran wrote:
> Is the comment in target/arm/op_helper.c:397 still relevant?
>
> uint32_t HELPER(cpsr_read)(CPUARMState *env)
> {
> /*
> * We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr.
> * This is convenient for populating SPSR_ELx, but must be
> * hidden from aarch32 mode, where it is not visible.
> *
> * TODO: ARMv8.4-DIT -- need to move SS somewhere else.
> */
> return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS);
> }
I forgot about this. So we can't "just" store DIT in uncached_cpsr.
I'll let Peter weigh in, but I think it makes sense to move the SS bit
somewhere else (e.g. env->pstate) and merge it into SPSR_ELx upon interrupt.
While what we're doing here is convenient, it's not architectural, and it would
be better to follow GetPSRFromPSTATE pseudocode.
r~
RE: [PATCH v15 0/4] Add Versal usb model
Hi, > -Original Message- > From: Peter Maydell > Sent: Friday, December 11, 2020 9:40 PM > To: Sai Pavan Boddu > Cc: Markus Armbruster ; Marc-André Lureau > ; Paolo Bonzini ; > Gerd Hoffmann ; Edgar Iglesias ; > Francisco Eduardo Iglesias ; Alistair Francis > ; Eduardo Habkost ; Ying > Fang ; Philippe Mathieu-Daudé > ; Vikram Garhwal ; Paul Zimmerman > ; Sai Pavan Boddu ; QEMU > Developers > Subject: Re: [PATCH v15 0/4] Add Versal usb model > > On Thu, 3 Dec 2020 at 19:18, Sai Pavan Boddu > wrote: > > > > This patch series adds dwc3 usb controller to versal SOC. > > > > > > Applied to target-arm.next, thanks. Thanks. Regards, Sai Pavan > > -- PMM
Re: [PATCH v5 00/11] hvf: Implement Apple Silicon Support
Patchew URL: https://patchew.org/QEMU/20201211151300.85322-1-ag...@csgraf.de/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201211151300.85322-1-ag...@csgraf.de
Subject: [PATCH v5 00/11] hvf: Implement Apple Silicon Support
=== 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/20201210203549.379-1-peter.mayd...@linaro.org ->
patchew/20201210203549.379-1-peter.mayd...@linaro.org
* [new tag] patchew/20201211151300.85322-1-ag...@csgraf.de ->
patchew/20201211151300.85322-1-ag...@csgraf.de
- [tag update] patchew/20201211152426.350966-1-th...@redhat.com ->
patchew/20201211152426.350966-1-th...@redhat.com
Switched to a new branch 'test'
56f4c6b hvf: arm: Implement -cpu host
b5c4db1 hvf: arm: Add support for GICv3
6c0280c arm/hvf: Add a WFI handler
26b170e arm: Add Hypervisor.framework build target
a8820bd hvf: Add Apple Silicon support
d0b5c38 hvf: Simplify post reset/init/loadvm hooks
d3c68ab arm: Set PSCI to 0.2 for HVF
360f48e hvf: Introduce hvf vcpu struct
87a09e8 hvf: Move common code out
ac925f0 hvf: x86: Remove unused definitions
c77ce16 hvf: Add hypervisor entitlement to output binaries
=== OUTPUT BEGIN ===
1/11 Checking commit c77ce16832fb (hvf: Add hypervisor entitlement to output
binaries)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#18:
new file mode 100644
total: 0 errors, 1 warnings, 62 lines checked
Patch 1/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/11 Checking commit ac925f0aaf18 (hvf: x86: Remove unused definitions)
3/11 Checking commit 87a09e81f925 (hvf: Move common code out)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41:
new file mode 100644
total: 0 errors, 1 warnings, 1054 lines checked
Patch 3/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/11 Checking commit 360f48eb839f (hvf: Introduce hvf vcpu struct)
WARNING: line over 80 characters
#142: FILE: target/i386/hvf/hvf.c:213:
+wvmcs(cpu->hvf->fd, VMCS_ENTRY_CTLS,
cap2ctrl(hvf_state->hvf_caps->vmx_cap_entry,
ERROR: "(foo*)" should be "(foo *)"
#750: FILE: target/i386/hvf/x86hvf.c:85:
+if (hv_vcpu_write_fpstate(cpu_state->hvf->fd, (void*)xsave, 4096)) {
ERROR: "(foo*)" should be "(foo *)"
#831: FILE: target/i386/hvf/x86hvf.c:167:
+if (hv_vcpu_read_fpstate(cpu_state->hvf->fd, (void*)xsave, 4096)) {
total: 2 errors, 1 warnings, 996 lines checked
Patch 4/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/11 Checking commit d3c68ab7fa96 (arm: Set PSCI to 0.2 for HVF)
6/11 Checking commit d0b5c38651d5 (hvf: Simplify post reset/init/loadvm hooks)
7/11 Checking commit a8820bd60f6a (hvf: Add Apple Silicon support)
WARNING: architecture specific defines should be avoided
#48: FILE: accel/hvf/hvf-cpus.c:61:
+#ifdef __aarch64__
WARNING: architecture specific defines should be avoided
#59: FILE: accel/hvf/hvf-cpus.c:335:
+#ifdef __aarch64__
WARNING: architecture specific defines should be avoided
#91: FILE: include/sysemu/hvf_int.h:15:
+#ifdef __aarch64__
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#117:
new file mode 100644
WARNING: line over 80 characters
#579: FILE: target/arm/hvf/hvf.c:458:
+hv_vcpu_set_pending_interrupt(cpu->hvf->fd, HV_INTERRUPT_TYPE_FIQ,
true);
WARNING: line over 80 characters
#584: FILE: target/arm/hvf/hvf.c:463:
+hv_vcpu_set_pending_interrupt(cpu->hvf->fd, HV_INTERRUPT_TYPE_IRQ,
true);
total: 0 errors, 6 warnings, 691 lines checked
Patch 7/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/11 Checking commit 26b170ee618a (arm: Add Hypervisor.framework build target)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#49:
new file mode 100644
total: 0 errors, 1 warnings, 36 lines checked
Patch 8/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/11 Checking commit 6c0280c8b649 (arm/hvf: Add a WFI handler)
10/11 Checking commit b5c4db10905e (hvf: arm: Add support for GICv3)
11/11 Checking commit 56f4c6b7ff84 (hvf: arm: Implement -cpu host)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20201211151300.8
Re: [PATCH v11 18/25] cpu: Move synchronize_from_tb() to tcg_ops
On 12/11/20 7:51 PM, Claudio Fontana wrote: > On 12/11/20 7:26 PM, Philippe Mathieu-Daudé wrote: >> On 12/11/20 7:22 PM, Richard Henderson wrote: >>> On 12/11/20 12:15 PM, Claudio Fontana wrote: Should I return this file to the original state (without the extra #includes that pretend it to be a standalone header file, and call it tcg-cpu-ops.h.inc ? >>> >>> If this header can work with qemu/typedefs.h, then no, because the >>> circularity >>> has been resolved. Otherwise, yes. >> >> My editor got confused with TranslationBlock, which is why I asked >> to include its declaration. >> >> Easier to forward-declare TranslationBlock in qemu/typedefs.h? >> >> Regards, >> >> Phil. >> > > Hello Philippe, > > ok you propose to move the existing fwd declaration of TranslationBlock from > cpu.h to qemu/typedefs.h . I'll let that answer to Richard =) > > And what about #include "exec/memattrs.h"? > > I assume you propose to put struct MemTxAttrs there as a fwd declaration too, > > and this concludes our experiment here? Well, there is no circular problem here, right? (and Richard already reviewed patch #24 :P ) > > Thanks, > > Claudio >
Re: [PULL 0/8] Ui 20201211 patches
On Fri, 11 Dec 2020 at 09:16, Gerd Hoffmann wrote: > > The following changes since commit 2ecfc0657afa5d29a373271b342f704a1a3c6737: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2020-12-10' > into staging (2020-12-10 17:01:05 +) > > are available in the Git repository at: > > git://git.kraxel.org/qemu tags/ui-20201211-pull-request > > for you to fetch changes up to 2951106143f6cf20b3a0e4f2078721503fe6418a: > > sdl2: Add extra mouse buttons (2020-12-11 08:06:40 +0100) > > > ui/console ui_info tweaks. > ui/vnc: alpha cursor support. > ui/vnc: locking fixes. > ui/sdl: add extra mouse buttons. > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0 for any user-visible changes. -- PMM
Re: [PATCH v11 18/25] cpu: Move synchronize_from_tb() to tcg_ops
On 12/11/20 7:26 PM, Philippe Mathieu-Daudé wrote: > On 12/11/20 7:22 PM, Richard Henderson wrote: >> On 12/11/20 12:15 PM, Claudio Fontana wrote: >>> Should I return this file to the original state (without the extra >>> #includes that pretend it to be a standalone header file, >>> and call it >>> >>> tcg-cpu-ops.h.inc >>> >>> ? >> >> If this header can work with qemu/typedefs.h, then no, because the >> circularity >> has been resolved. Otherwise, yes. > > My editor got confused with TranslationBlock, which is why I asked > to include its declaration. > > Easier to forward-declare TranslationBlock in qemu/typedefs.h? > > Regards, > > Phil. > Hello Philippe, ok you propose to move the existing fwd declaration of TranslationBlock from cpu.h to qemu/typedefs.h . And what about #include "exec/memattrs.h"? I assume you propose to put struct MemTxAttrs there as a fwd declaration too, and this concludes our experiment here? Thanks, Claudio
Re: [PATCH v5 6/7] tcg: implement JIT for iOS and Apple Silicon
Sounds good, I will make that change in the next version. -j On Fri, Dec 11, 2020 at 4:36 AM Stefan Hajnoczi wrote: > > On Fri, Dec 11, 2020 at 10:54 AM Alexander Graf wrote: > > On 25.11.20 03:08, Joelle van Dyne wrote: > > > A lot of users of UTM are on iOS 13 (a large number of devices only > > > have jailbreak for iOS 13 and below), but if the QEMU community thinks > > > it's better that way, we are willing to compromise. > > > > > > I think it would make merging much more straight forward if we could > > keep RWX toggling to the publicly released API. So yes, please adapt it. > > In UTM, you can still carry a tiny downstream patch that implements the > > API through your reverse engineered code for iOS 13, no? > > Alex, you're awesome! Thanks for finding a way to avoid the > reverse-engineered code. With that change we don't need to go through > a legal review and it makes merging this much simpler. > > Stefan
Re: Some performance numbers for virtiofs, DAX and virtio-9p
On Fri, Dec 11, 2020 at 06:29:56PM +, Dr. David Alan Gilbert wrote: [..] > > > > > > Could we measure at what point does a large window size actually make > > > performance worse? > > > > Will do. Will run tests with varying window sizes (small to large) > > and see how does it impact performance for same workload with > > same guest memory. > > I wonder how realistic it is though; it makes some sense if you have a > scenario like a fairly small root filesystem - something tractable; but > if you have a large FS you're not realistically going to be able to set > the cache size to match it - that's why it's a cache! I think its more about active dataset size and not necessarily total FS size. FS might be big but if application is not accessing all of the in reasonabl small time window, then it does not matter. What worries me most is that cost of reclaim of a dax range seems too high (or keeps the process blocked for long enogh), that it kills the performance. I will need to revisit the reclaim path and see if I can optimize something. Vivek
Re: [PATCH v2 2/8] gitlab: include aarch64-softmmu and ppc64-softmmu cross-system-build
On 12/10/20 4:04 PM, Alex Bennée wrote: Otherwise we miss coverage of KVM support in the cross build. To balance it out add arm-softmmu (no kvm, subset of aarch64), cris-softmmu and ppc-softmmu to the exclude list which do get coverage elsewhere. Signed-off-by: Alex Bennée --- .gitlab-ci.d/crossbuilds.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Wainer dos Santos Moschetta diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml index bd6473a75a..fcc1b95290 100644 --- a/.gitlab-ci.d/crossbuilds.yml +++ b/.gitlab-ci.d/crossbuilds.yml @@ -7,9 +7,9 @@ - cd build - PKG_CONFIG_PATH=$PKG_CONFIG_PATH ../configure --enable-werror $QEMU_CONFIGURE_OPTS --disable-user ---target-list-exclude="aarch64-softmmu i386-softmmu microblaze-softmmu - mips-softmmu mipsel-softmmu mips64-softmmu ppc64-softmmu sh4-softmmu - xtensa-softmmu" +--target-list-exclude="arm-softmmu cris-softmmu i386-softmmu + microblaze-softmmu mips-softmmu mipsel-softmmu mips64-softmmu + ppc-softmmu sh4-softmmu xtensa-softmmu" - make -j$(expr $(nproc) + 1) all check-build # Job to cross-build specific accelerators.
Re: [PATCH 19/20] block: Use GString instead of QString to build filenames
11.12.2020 20:11, Markus Armbruster wrote: QString supports modifying its string, but it's quite limited: you can only append. Just one caller remains: bdrv_parse_filename_strip_prefix() uses it just for building an initial string. Change it to do build the initial string with GString. This is another step towards making QString immutable. Cc: Kevin Wolf Cc: Max Reitz Cc:qemu-bl...@nongnu.org Signed-off-by: Markus Armbruster Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
