Re: [PATCH 0/2] Zynq 7000 SoC improvements
On Fri, 17 May 2024 at 09:31, Sebastian Huber wrote: > > Hello, > > is the mailing list the right place for contributions like this? Yes it is, and this is on my todo list to review. Sorry for not getting back to you earlier, but I was on holiday last week and at a conference this week. I hope to be able to start working through my code review backlog when I'm at my desk again next week :-) -- PMM
Re: [PATCH v2 1/3] docs: introduce dedicated page about code provenance / sign-off
On Thu, 16 May 2024 at 18:34, Michael S. Tsirkin wrote: > > On Thu, May 16, 2024 at 06:29:39PM +0100, Peter Maydell wrote: > > On Thu, 16 May 2024 at 17:22, Daniel P. Berrangé > > wrote: > > > > > > Currently we have a short paragraph saying that patches must include > > > a Signed-off-by line, and merely link to the kernel documentation. > > > The linked kernel docs have a lot of content beyond the part about > > > sign-off an thus are misleading/distracting to QEMU contributors. > > > > Thanks for this -- I've felt for ages that it was a bit awkward > > that we didn't have a good place to link people to for the fuller > > explanation of this. > > > > > This introduces a dedicated 'code-provenance' page in QEMU talking > > > about why we require sign-off, explaining the other tags we commonly > > > use, and what to do in some edge cases. > > > > The version of the kernel SubmittingPatches we used to link to > > includes the text "sorry, no pseudonyms or anonymous contributions". > > This new documentation doesn't say anything either way about > > our approach to pseudonyms. I think we should probably say > > something, but I don't know if we have an in-practice consensus > > there, so maybe we should approach that as a separate change on > > top of this patch. > > > Well given we referred to kernel previously then I guess that's > the concensus, no? AIUI the kernel devs have changed their point of view on the pseudonym question, so it's a question of whether we were deliberately referring to that specific revision of the kernel's practice because we agreed with it or just by chance... https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=d4563201f33a022fc0353033d9dfeb1606a88330 is where the kernel changed to saying merely "no anonymous contributions", dropping the 'pseudonyms' part. -- PMM
Re: [PATCH v2 0/3] docs: define policy forbidding use of "AI" / LLM code generators
On Thu, 16 May 2024 at 18:20, Michael S. Tsirkin wrote: > > On Thu, May 16, 2024 at 05:22:27PM +0100, Daniel P. Berrangé wrote: > > AFAICT at its current state of (im)maturity the question of licensing > > of AI code generator output does not have a broadly accepted / settled > > legal position. This is an inherant bias/self-interest from the vendors > > promoting their usage, who tend to minimize/dismiss the legal questions. > > >From my POV, this puts such tools in a position of elevated legal risk. > > > > Given the fuzziness over the legal position of generated code from > > such tools, I don't consider it credible (today) for a contributor > > to assert compliance with the DCO terms (b) or (c) (which is a stated > > pre-requisite for QEMU accepting patches) when a patch includes (or is > > derived from) AI generated code. > > > > By implication, I think that QEMU must (for now) explicitly decline > > to (knowingly) accept AI generated code. > > > > Perhaps a few years down the line the legal uncertainty will have > > reduced and we can re-evaluate this policy. > At this junction, the code generated by these tools is of such > quality that I really won't expect it to pass even cursory code > review. I disagree, I think that in at least some cases they can produce code that would pass our quality bar, especially with human supervision and editing after the fact. If the problem was merely "LLMs tend to produce lousy output" then we wouldn't need to write anything new -- we already have a process for dealing with bad patches, which is to say we do code review and suggest changes or simply reject the patches. What we *don't* have any process to handle is the legal uncertainties that Dan outlines above. -- PMM
Re: [PATCH v2 1/3] docs: introduce dedicated page about code provenance / sign-off
On Thu, 16 May 2024 at 17:22, Daniel P. Berrangé wrote: > > Currently we have a short paragraph saying that patches must include > a Signed-off-by line, and merely link to the kernel documentation. > The linked kernel docs have a lot of content beyond the part about > sign-off an thus are misleading/distracting to QEMU contributors. Thanks for this -- I've felt for ages that it was a bit awkward that we didn't have a good place to link people to for the fuller explanation of this. > This introduces a dedicated 'code-provenance' page in QEMU talking > about why we require sign-off, explaining the other tags we commonly > use, and what to do in some edge cases. The version of the kernel SubmittingPatches we used to link to includes the text "sorry, no pseudonyms or anonymous contributions". This new documentation doesn't say anything either way about our approach to pseudonyms. I think we should probably say something, but I don't know if we have an in-practice consensus there, so maybe we should approach that as a separate change on top of this patch. So for this patch: Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH 2/4] MAINTAINERS: drop usb maintainership
On Thu, 16 May 2024 at 13:04, Gerd Hoffmann wrote: > > Remove myself from usb entries. > Flip status to "Orphan" for entries which have nobody else listed. > diff --git a/MAINTAINERS b/MAINTAINERS > index 7f52e2912fc3..d81376f84746 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS Thanks for your time and efforts in looking after these components in the past. > @@ -2140,8 +2140,7 @@ F: tests/qtest/fuzz-sdcard-test.c > F: tests/qtest/sdhci-test.c > > USB > -M: Gerd Hoffmann > -S: Odd Fixes > +S: Orphan > F: hw/usb/* > F: stubs/usb-dev-stub.c > F: tests/qtest/usb-*-test.c Does RedHat have any corporate interest in finding somebody else to look after the USB components in future ? thanks -- PMM
Re: [PULL 0/5] loongarch-to-apply queue
On Thu, 16 May 2024 at 10:12, Song Gao wrote: > > The following changes since commit 922582ace2df59572a671f5c0c5c6c5c706995e5: > > Merge tag 'pull-hppa-20240515' of https://gitlab.com/rth7680/qemu into > staging (2024-05-15 11:46:58 +0200) > > are available in the Git repository at: > > https://gitlab.com/gaosong/qemu.git tags/pull-loongarch-20240516 > > for you to fetch changes up to d55d16700a2e2b36c7e34724d4d77f4a75c5243a: > > target/loongarch/kvm: fpu save the vreg registers high 192bit (2024-05-16 > 16:32:35 +0800) > > > pull-loongarch-20240516 > > > Bibo Mao (3): > hw/loongarch: Add compat machine for 9.1 > hw/loongarch: Remove minimum and default memory size > tests: Add migration test for loongarch64 RTH: I had a comment about adding the versioned machine type, so we should hold off on applying this until that is resolved, I think. thanks -- PMM
Re: [PULL 1/5] hw/loongarch: Add compat machine for 9.1
On Thu, 16 May 2024 at 10:12, Song Gao wrote: > > From: Bibo Mao > > Since migration test case requires compat machine type support, > compat machine is added for qemu 9.1 here. This is not a good justification for adding versioned machine types. Adding versioned machine types is putting a massive maintenance burden on yourself from now onward: * every release needs a new machine type adding * every change to the board model or to any devices it uses needs to be assessed for cross-version compatibility issues, and if necessary extra properties must be added so old versions of the board model keep the legacy backwards-compatible behaviour * migration compatibility must be maintained You really do not want to do this until you decide that your architecture and machine type are at a level where this is a support guarantee you want to provide to users. "It lets us run a test case" is not a good reason to do it, and I strongly recommend you do not add versioned machine types just for that. thanks -- PMM
Re: [PATCH v2 1/3] qtest: allow SPCR acpi table changes
On Mon, 13 May 2024 at 11:36, Alistair Francis wrote: > > On Mon, May 13, 2024 at 4:32 PM Michael S. Tsirkin wrote: > > > > On Mon, May 13, 2024 at 01:55:50PM +1000, Alistair Francis wrote: > > > On Tue, May 7, 2024 at 3:24 PM Sia Jee Heng > > > wrote: > > > > > > Can you describe why you are doing this and that it will be reverted > > > in the commit message? > > > > > > Alistair > > > > What motivation are you asking? This follows the normal acpi test update > > procedure. > > I find it clearer to have commits describe that they are disabling > tests for a specific reason. That way it's easier to track what's > going on. > > If ACPI test updates don't usually do that then that's fine with me The only reason for the existence of this ignore-these-blobs file is for the purpose of the commit sequence: * add the blobs to the whitelist * make a change that alters what the expected blobs are * regenerate the golden-reference blobs and remove the items from the whitelist So we don't usually say much in the commit that is adding a blob to the whitelist. thanks -- PMM
Re: [PATCH 0/3] Assorted fixes for PMU
On Mon, 29 Apr 2024 at 20:29, Atish Patra wrote: > > This series contains few miscallenous fixes related to hpmcounters > and related code. The first patch fixes an issue with cycle/instret > counters overcouting while the remaining two are more for specification > compliance. I've noticed a number of riscv patchsets from various people recently where the subject line for the cover letter doesn't include any indication that it's a riscv related patchset. For instance this one is just "Assorted fixes for PMU", which could easily be an Arm PMU series. Could you all try to make sure that cover letter subject lines indicate the architecture or other subcomponent they affect, please? This helps people who are skimming over the mailing list looking for patches relevant to them. thanks -- PMM
Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT
On Thu, 9 May 2024 at 19:17, Cord Amfmgm wrote: > > > > On Thu, May 9, 2024 at 12:48 PM Peter Maydell > wrote: >> >> On Wed, 8 May 2024 at 16:29, Cord Amfmgm wrote: >> > On Wed, May 8, 2024 at 3:45 AM Thomas Huth wrote: >> >> >> >> Your Signed-off-by line does not match the From: line ... could you please >> >> fix this? (see >> >> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#patch-emails-must-include-a-signed-off-by-line >> >> , too) >> > >> > >> > I'll submit the new patch request with my pseudonym in the From: and >> > Signed-off-by: lines, per your request. Doesn't matter to me. However, >> > this arises simply because I don't give gmail my real name - >> > https://en.wikipedia.org/wiki/Nymwars >> >> I'm confused now. Of the two names you've used in this >> patch (Cord Amfmgm and David Hubbard), are they both >> pseudonyms, or is one a pseudonym and one your real name? >> > > Hi Peter, > > I am attempting to submit a small patch. For context, I'm getting broader > attention now because apparently OHCI is one of the less used components of > qemu and maybe the review process was taking a while. That's relevant because > I wasn't able to get prompt feedback and am now choosing what appears to be > the most expeditious approach -- all I want is to get this patch done and be > out of your hair. If Thomas Huth wants me to use a consistent name, have I > not complied? Are you asking out of curiosity or is there a valid reason why > I should answer your question in order to get the patch submitted? Would you > like to have a friendly chat over virtual coffee sometime (but off-list)? > > If you could please clarify I'm sure the answer is an easy one. I'm asking because our basic expected position is "commits are from the submitter's actual name, not a pseudonym". Obviously we can't tell if people use a consistent plausible looking pseudonym whether that corresponds to their real-world name or not, but if you have a real name you're happy to attach to this patch and are merely using a pseudonym for Google email, then the resubmit of this patch didn't seem to me to do that. i.e. I was expecting the change to be "make the patch From: match the Signed-off-by line", not "make the Signed-off-by line match the patch From:". (For avoidance of doubt, we don't care about the email From: line, which is distinct from the commit message From: i.e. author.) So I was essentially asking "did you mean to do this, or did you misunderstand what we were asking for?". On the question of the actual patch, I'll try to get to it if Gerd doesn't first (though I have a conference next week so it might be the week after). The main thing I need to chase down is whether it's OK to call usb_packet_addbuf() with a zero length or not. thanks -- PMM
Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT
On Wed, 8 May 2024 at 16:29, Cord Amfmgm wrote: > On Wed, May 8, 2024 at 3:45 AM Thomas Huth wrote: >> >> Your Signed-off-by line does not match the From: line ... could you please >> fix this? (see >> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#patch-emails-must-include-a-signed-off-by-line >> , too) > > > I'll submit the new patch request with my pseudonym in the From: and > Signed-off-by: lines, per your request. Doesn't matter to me. However, this > arises simply because I don't give gmail my real name - > https://en.wikipedia.org/wiki/Nymwars I'm confused now. Of the two names you've used in this patch (Cord Amfmgm and David Hubbard), are they both pseudonyms, or is one a pseudonym and one your real name? thanks -- PMM
Re: [PATCH] hw/clock: Expose 'freq-hz' QOM property
On Wed, 8 May 2024 at 15:13, Philippe Mathieu-Daudé wrote: > > Expose the clock frequency via the QOM 'freq-hz' property, > as it might be useful for QTests. > > HMP example: > > $ qemu-system-mips -S -monitor stdio -M mipssim > (qemu) qom-get /machine/cpu-refclk freq-hz > 1200 > > Inspired-by: Inès Varhol > Signed-off-by: Philippe Mathieu-Daudé So I have a couple of thoughts here: (1) if this is intended for qtests, would exposing the period (i.e. QOM equivalent of clock_get() rather than clock_get_hz()) be better? A Hz figure has rounding so it's not as accurate. (2) We should document this in clocks.rst; I guess we want to say "only intended for use in qtests" (i.e. if you're part of QEMU use the existing function interface, not this). thanks -- PMM
Re: [PATCH] hw/loongarch/virt: Fix memory leak
On Tue, 7 May 2024 at 10:52, Michael Tokarev wrote:
>
> 07.05.2024 05:22, Song Gao wrote:
>
> > for (i = 1; i < nb_numa_nodes; i++) {
> > MemoryRegion *nodemem = g_new(MemoryRegion, 1);
> > -ramName = g_strdup_printf("loongarch.node%d.ram", i);
> > +g_autofree char *ramName = g_strdup_printf("loongarch.node%d.ram",
> > i);
>
> Can't this be a fixed-size buffer on stack?
No, this is a really bad idea. It's a pain to audit that the
array really doesn't get overwritten, and if the string we want
to write changes, now we have to re-count characters to decide
if we need to increase the size of the array. The memory allocation
on the heap here is a tiny overhead that we only incur at startup.
The g_autofree approach is much better.
For this version of the patch:
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH v2] hw/loongarch/virt: Fix memory leak
On Wed, 8 May 2024 at 03:30, Song Gao wrote:
>
> The char pointer 'ramName' point to a block of memory, but never free it.
> Use a small fixed-size buffer for 'ramName'.
>
> Resolves: Coverity CID 1544773
>
> Fixes: 0cf1478d6 ("hw/loongarch: Add numa support")
> Signed-off-by: Song Gao
> ---
> v2:
> - Use a small fixed-size buffer for 'ramName'.
> - Link to V1:
> https://patchew.org/QEMU/20240507022239.3113987-1-gaos...@loongson.cn/
>
> hw/loongarch/virt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index c0999878df..ee690ad981 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -887,7 +887,7 @@ static void loongarch_init(MachineState *machine)
> const CPUArchIdList *possible_cpus;
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> CPUState *cpu;
> -char *ramName = NULL;
> +char ramName[32];
Please don't use fixed-size char arrays for writing strings like this.
> if (!cpu_model) {
> cpu_model = LOONGARCH_CPU_TYPE_NAME("la464");
> @@ -946,7 +946,7 @@ static void loongarch_init(MachineState *machine)
>
> for (i = 1; i < nb_numa_nodes; i++) {
> MemoryRegion *nodemem = g_new(MemoryRegion, 1);
> -ramName = g_strdup_printf("loongarch.node%d.ram", i);
> +sprintf(ramName, "loongarch.node%d.ram", i);
The nicest way to fix this is to use the g_autofree mechanism
so the memory is automatically freed at the end of the block:
g_autofree char *ramName = g_strdup_printf(...);
> memory_region_init_alias(nodemem, NULL, ramName, machine->ram,
> offset, numa_info[i].node_mem);
> memory_region_add_subregion(address_space_mem, phyAddr, nodemem);
thanks
-- PMM
Re: [PATCH] target/loongarch/kvm: Fix VM recovery from disk failures
On Tue, 7 May 2024 at 16:47, Peter Xu wrote: > > On Tue, May 07, 2024 at 04:12:34PM +0800, gaosong wrote: > > Just remove CONIFG_KVM would be OK? > > You're the loongarch maintainer so I'd say your call. :) > > If you're not yet sure, IMHO we should go with the simplest, which is the > original oneliner patch. The original patch needs to also bump the version numbers when it adds the new field. Even when we do not wish to maintain migration compatibility, bumping the version number means that users get a (more or less) helpful error message if they try an unsupported cross-version migration, rather than weird behaviour. thanks -- PMM
Re: [RFC PATCH 0/1] pci: allocate a PCI ID for RISC-V IOMMU
On Fri, 3 May 2024 at 13:43, Daniel Henrique Barboza wrote: > > Hi, > > In this RFC I want to check with Gerd and others if it's ok to add a PCI > id for the RISC-V IOMMU device. It's currently under review in [1]. The > idea is to fold this patch into the RISC-V IOMMU series if we're all ok > with this change. My question here would be "why is this risc-v specific?" (and more generally "what is this for?" -- the cover letter and patch and documentation page provide almost no information about what this device is and why it needs to exist rather than using either virtio-iommu or else a model of a real hardware IOMMU.) thanks -- PMM
Re: [PATCH] misc: Use QEMU header path relative to include/ directory
On Tue, 7 May 2024 at 15:28, Philippe Mathieu-Daudé wrote: > > QEMU headers are relative to the include/ directory, > not to the project root directory. Remove "include/". > > See also: > https://www.qemu.org/docs/master/devel/style.html#include-directives > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/audio/virtio-snd.c | 2 +- > hw/rtc/ls7a_rtc.c | 2 +- > target/i386/gdbstub.c | 2 +- > tests/qtest/nvme-test.c | 2 +- > tests/qtest/ufs-test.c | 2 +- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] configure: quote -D options that are passed to meson
On Tue, 7 May 2024 at 11:50, Paolo Bonzini wrote: > > Ensure that they go through unmodified, instead of removing one layer > of quoting. Do you have an example of what goes wrong that we could mention in the commit message ? thanks -- PMM
Re: [PATCH] Fixes: Indentation using TABs and improve formatting
On Mon, 6 May 2024 at 07:20, Tanmay wrote: > > Hi, > > I have added a patch inline that fixes indentation and formatting for some > files as listed in https://gitlab.com/qemu-project/qemu/-/issues/373. > > Thanks, > Tanmay Hi; thanks for this patch. Unfortunately there seem to be some problems with the formatting, which mean it doesn't apply correctly. In particular you've sent it as a plain/text + HTML email, and something (probably your mail client) is wrapping long lines. I can usually fix something like this up on my end for a first time submitter, but in this case the patch is just way too big for that to be practical. I generally don't recommend trying to send patch emails directly through the gmail web UI -- it unfortunately mangles them too much. Personally I use git-send-email; it is a bit awkward to set up, but https://git-send-email.io/ has a nice step-by-step guide including specific details on how to configure it to send via Gmail. > From 46026574821c46804111eea6607a1b39314b7abe Mon Sep 17 00:00:00 2001 > From: Tanmay Patil > Date: Sat, 25 Nov 2023 00:53:54 +0530 > Subject: [PATCH] Fixes: Indentation using TABs and improve formatting > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/373 > >Files changed: > - hw/arm/boot.c > - hw/char/omap_uart.c > - hw/dma/pxa2xx_dma.c > - hw/gpio/omap_gpio.c > - hw/gpio/zaurus.c > - hw/input/tsc2005.c > - hw/input/tsc210x.c > - hw/intc/omap_intc.c > - hw/misc/cbus.c > - hw/misc/omap_clk.c > - hw/misc/omap_l4.c > - hw/misc/omap_sdrc.c > - hw/misc/omap_tap.c > - hw/sd/omap_mmc.c > - hw/sd/pxa2xx_mmci.c > - hw/timer/omap_gptimer.c > - hw/timer/omap_synctimer.c > - hw/timer/pxa2xx_timer.c > - include/hw/arm/pxa.h > - include/hw/arm/sharpsl.h > - include/hw/arm/soc_dma.h > - tcg/arm/tcg-target.h > > Signed-off-by: Tanmay Patil > --- > hw/arm/boot.c | 8 +- > hw/char/omap_uart.c | 49 +- > hw/dma/pxa2xx_dma.c | 198 > hw/gpio/omap_gpio.c | 243 +- > hw/gpio/zaurus.c | 61 +-- > hw/input/tsc2005.c| 130 ++--- > hw/input/tsc210x.c| 442 + > hw/intc/omap_intc.c | 261 +- > hw/misc/cbus.c| 202 > hw/misc/omap_clk.c| 999 +++--- > hw/misc/omap_l4.c | 21 +- > hw/misc/omap_sdrc.c | 135 +++--- > hw/misc/omap_tap.c| 28 +- > hw/sd/omap_mmc.c | 208 > hw/sd/pxa2xx_mmci.c | 149 +++--- > hw/timer/omap_gptimer.c | 126 ++--- > hw/timer/omap_synctimer.c | 7 +- > hw/timer/pxa2xx_timer.c | 279 ++- > include/hw/arm/pxa.h | 100 ++-- > include/hw/arm/sharpsl.h | 2 +- > include/hw/arm/soc_dma.h | 4 +- > tcg/arm/tcg-target.h | 4 +- > 22 files changed, 1903 insertions(+), 1753 deletions(-) This is a lot of changes to do all in one commit, even though they're all just whitespace fixes. Could you split them up, please, into multiple patches that tackle fewer files at a time? thanks -- PMM
Re: [PATCH 3/4] hw/char: Add QOM property for STM32L4x5 USART clock frequency
On Mon, 6 May 2024 at 10:34, Philippe Mathieu-Daudé wrote:
>
> Hi,
>
> On 5/5/24 16:05, Inès Varhol wrote:
> > Signed-off-by: Inès Varhol
> > ---
> > hw/char/stm32l4x5_usart.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
> > index fc5dcac0c4..ee7727481c 100644
> > --- a/hw/char/stm32l4x5_usart.c
> > +++ b/hw/char/stm32l4x5_usart.c
> > @@ -26,6 +26,7 @@
> > #include "hw/clock.h"
> > #include "hw/irq.h"
> > #include "hw/qdev-clock.h"
> > +#include "qapi/visitor.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/qdev-properties-system.h"
> > #include "hw/registerfields.h"
> > @@ -523,6 +524,14 @@ static Property stm32l4x5_usart_base_properties[] = {
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> > +static void clock_freq_get(Object *obj, Visitor *v,
> > +const char *name, void *opaque, Error **errp)
> > +{
> > +Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
> > +uint32_t clock_freq_hz = clock_get_hz(s->clk);
> > +visit_type_uint32(v, name, _freq_hz, errp);
> > +}
> > +
> > static void stm32l4x5_usart_base_init(Object *obj)
> > {
> > Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
> > @@ -534,6 +543,9 @@ static void stm32l4x5_usart_base_init(Object *obj)
> > sysbus_init_mmio(SYS_BUS_DEVICE(obj), >mmio);
> >
> > s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
> > +
> > +object_property_add(obj, "clock-freq-hz", "uint32",
> > +clock_freq_get, NULL, NULL, NULL);
>
> Patch LGTM, but I wonder if registering QOM getter without setter
> is recommended. Perhaps we should encourage parity? In normal HW
> emulation we shouldn't update this clock externally, but thinking
> about testing, this could be interesting to introduce jitter.
object_property_add() with the set function NULL is fine,
and is documented to mean "property cannot be set". Attempts
to set it will be failed (in object_property_set()) with a
reasonable error.
But it's not clear to me why we want the property in the first
place -- we don't generally have devices which take a Clock
input have properties exposing its frequency. If we did want
that it would probably be better if we could do it generically
rather than by adding more boilerplate code to each device.
Mostly "frequency" properties on devices are for the case
where they *don't* have a Clock input and instead have
ad-hoc legacy handling where the board/SoC that creates the
device sets an integer property to define the input frequency
because it doesn't model the clock tree with Clock objects.
thanks
-- PMM
Re: [PATCH 1/4] hw/misc: Create STM32L4x5 SYSCFG clock
On Sun, 5 May 2024 at 15:06, Inès Varhol wrote:
>
> Signed-off-by: Inès Varhol
In general you should try to avoid commits with no commit message.
Sometimes there really isn't anything to say beyond what the
subject line is, but that should be the exception rather than
the usual thing.
> ---
> include/hw/misc/stm32l4x5_syscfg.h | 1 +
> hw/arm/stm32l4x5_soc.c | 2 ++
> hw/misc/stm32l4x5_syscfg.c | 26 ++
> 3 files changed, 29 insertions(+)
>
> diff --git a/include/hw/misc/stm32l4x5_syscfg.h
> b/include/hw/misc/stm32l4x5_syscfg.h
> index 23bb564150..c450df2b9e 100644
> --- a/include/hw/misc/stm32l4x5_syscfg.h
> +++ b/include/hw/misc/stm32l4x5_syscfg.h
> @@ -48,6 +48,7 @@ struct Stm32l4x5SyscfgState {
> uint32_t swpr2;
>
> qemu_irq gpio_out[GPIO_NUM_PINS];
> +Clock *clk;
> };
>
> #endif
> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
> index 38f7a2d5d9..fb2afa6cfe 100644
> --- a/hw/arm/stm32l4x5_soc.c
> +++ b/hw/arm/stm32l4x5_soc.c
> @@ -236,6 +236,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
> Error **errp)
>
> /* System configuration controller */
> busdev = SYS_BUS_DEVICE(>syscfg);
> +qdev_connect_clock_in(DEVICE(>syscfg), "clk",
> +qdev_get_clock_out(DEVICE(&(s->rcc)), "syscfg-out"));
> if (!sysbus_realize(busdev, errp)) {
> return;
> }
> diff --git a/hw/misc/stm32l4x5_syscfg.c b/hw/misc/stm32l4x5_syscfg.c
> index a5a1ce2680..a82864c33d 100644
> --- a/hw/misc/stm32l4x5_syscfg.c
> +++ b/hw/misc/stm32l4x5_syscfg.c
> @@ -26,6 +26,10 @@
> #include "trace.h"
> #include "hw/irq.h"
> #include "migration/vmstate.h"
> +#include "hw/clock.h"
> +#include "hw/qdev-clock.h"
> +#include "qapi/visitor.h"
> +#include "qapi/error.h"
> #include "hw/misc/stm32l4x5_syscfg.h"
> #include "hw/gpio/stm32l4x5_gpio.h"
>
> @@ -202,6 +206,14 @@ static void stm32l4x5_syscfg_write(void *opaque, hwaddr
> addr,
> }
> }
>
> +static void clock_freq_get(Object *obj, Visitor *v,
> +const char *name, void *opaque, Error **errp)
> +{
> +Stm32l4x5SyscfgState *s = STM32L4X5_SYSCFG(obj);
> +uint32_t clock_freq_hz = clock_get_hz(s->clk);
> +visit_type_uint32(v, name, _freq_hz, errp);
> +}
> +
> static const MemoryRegionOps stm32l4x5_syscfg_ops = {
> .read = stm32l4x5_syscfg_read,
> .write = stm32l4x5_syscfg_write,
> @@ -225,6 +237,18 @@ static void stm32l4x5_syscfg_init(Object *obj)
> qdev_init_gpio_in(DEVICE(obj), stm32l4x5_syscfg_set_irq,
>GPIO_NUM_PINS * NUM_GPIOS);
> qdev_init_gpio_out(DEVICE(obj), s->gpio_out, GPIO_NUM_PINS);
> +s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
> +object_property_add(obj, "clock-freq-hz", "uint32", clock_freq_get, NULL,
> +NULL, NULL);
Why do we need this property? The clock on this device is an input,
so the device doesn't control its frequency.
> +}
> +
> +static void stm32l4x5_syscfg_realize(DeviceState *dev, Error **errp)
> +{
> +Stm32l4x5SyscfgState *s = STM32L4X5_SYSCFG(dev);
> +if (!clock_has_source(s->clk)) {
> +error_setg(errp, "SYSCFG: clk input must be connected");
> +return;
> +}
> }
>
> static const VMStateDescription vmstate_stm32l4x5_syscfg = {
> @@ -241,6 +265,7 @@ static const VMStateDescription vmstate_stm32l4x5_syscfg
> = {
> VMSTATE_UINT32(swpr, Stm32l4x5SyscfgState),
> VMSTATE_UINT32(skr, Stm32l4x5SyscfgState),
> VMSTATE_UINT32(swpr2, Stm32l4x5SyscfgState),
> +VMSTATE_CLOCK(clk, Stm32l4x5SyscfgState),
Adding a field to the vmstate means we must bump the version number,
since it's a migration compatibility break.
> VMSTATE_END_OF_LIST()
> }
> };
thanks
-- PMM
Re: [PATCH] hw/char: Correct STM32L4x5 usart register CR2 field ADD_0 size
On Sun, 5 May 2024 at 15:16, Inès Varhol wrote: > > Signed-off-by: Arnaud Minier > Signed-off-by: Inès Varhol Applied to target-arm.next, thanks. -- PMM
Re: [PATCH] hw/arm/npcm7xx: remove setting of mp-affinity
On Sat, 4 May 2024 at 15:17, Dorjoy Chowdhury wrote: > > The value of the mp-affinity property being set in npcm7xx_realize is > always the same as the default value it would have when arm_cpu_realizefn > is called if the property is not set here. So there is no need to set > the property value in npcm7xx_realize function. > > Signed-off-by: Dorjoy Chowdhury > --- Applied to target-arm.next, thanks. -- PMM
Re: [PATCH] hvf: arm: Fix encodings for ID_AA64PFR1_EL1 and debug System registers
On Fri, 3 May 2024 at 16:35, Zenghui Yu wrote:
>
> We wrongly encoded ID_AA64PFR1_EL1 using {3,0,0,4,2} in hvf_sreg_match[] so
> we fail to get the expected ARMCPRegInfo from cp_regs hash table with the
> wrong key.
>
> Fix it with the correct encoding {3,0,0,4,1}. With that fixed, the Linux
> guest can properly detect FEAT_SSBS2 on my M1 HW.
>
> All DBG{B,W}{V,C}R_EL1 registers are also wrongly encoded with op0 == 14.
> It happens to work because HVF_SYSREG(CRn, CRm, 14, op1, op2) equals to
> HVF_SYSREG(CRn, CRm, 2, op1, op2), by definition. But we shouldn't rely on
> it.
>
> Fixes: a1477da3ddeb ("hvf: Add Apple Silicon support")
> Signed-off-by: Zenghui Yu
> ---
Applied to target-arm.next, thanks. I threw in a cc:stable tag,
though it doesn't sound like the consequences of the bug are very
significant.
-- PMM
Re: [PATCH] gitlab: Drop --static from s390x linux-user build
On Mon, 6 May 2024 at 21:21, Richard Henderson wrote: > > The host does not have the correct libraries installed for static pie, > which causes host/guest address space interference for some tests. > There's no real gain from linking statically, so drop it. > > Signed-off-by: Richard Henderson > --- The lack of rcrt1.o seems to be a bug in the Ubuntu Jammy (22.04) libc6-dev packages. It is present in the versions in Mantic (23.04) and Noble (24.04). thanks -- PMM
Re: [PATCH V8 6/8] physmem: Add helper function to destroy CPU AddressSpace
On Tue, 7 May 2024 at 01:11, Salil Mehta wrote: > > > From: Peter Maydell > > Sent: Monday, May 6, 2024 10:29 AM > > To: Salil Mehta > > > > On Mon, 6 May 2024 at 10:06, Salil Mehta > > wrote: > > > > > > Hi Peter, > > > > > > Thanks for the review. > > > > > > > From: Peter Maydell When do we need to > > > > destroy a single address space in this way that means we need to > > > > keep a count of how many ASes the CPU currently has? The commit > > > > message talks about the case when we unrealize the whole CPU > > > > object, but in that situation you can just throw away all the ASes > > > > at once (eg by calling some > > > > cpu_destroy_address_spaces() function from > > cpu_common_unrealizefn()). > > > > > > > > > Yes, maybe, we can destroy all at once from common leg as well. I'd > > > prefer this to be done from the arch specific function for ARM to > > > maintain the clarity & symmetry of initialization and > > > un-initialization legs. For now, all of these address space destruction > > is > > happening in context to the arm_cpu_unrealizefn(). > > > > > > It’s a kind of trade-off between little more code and clarity but I'm > > > open to further suggestions. > > > > > > > > > > > > > > Also, if we're leaking stuff here by failing to destroy it, is that > > > > a problem for existing CPU types like x86 that we can already hotplug? > > > > > > No we are not. We are taking care of these in the ARM arch specific > > > legs within functions arm_cpu_(un)realizefn(). > > > > How can you be taking care of *x86* CPU types in the Arm unrealize? > > > Sorry, yes, I missed to reply that clearly. There was indeed a leak with x86 > reported > by Phillipe/David last year. In fact, Phillipe floated a patch last year for > this. > I thought it was fixed already as part of cpu_common_unrealize() but I just > checked and realized that the below proposed changed still isn’t part of the > mainline > > https://lore.kernel.org/qemu-devel/20230918160257.30127-9-phi...@linaro.org/ That seems like the right way to clean these up -- Philippe, do you want to fish that bugfix out of your big patchset and submit it separately ? > I can definitely add a common CPU AddressSpace destruction leg as part of this > patch if in case arch specific CPU unrealize does not cleans up its CPU > AddressSpace? Arch-specific CPU unrealize shouldn't need to do anything -- if we fix this similarly to Philippe's patch above then that will do the cleanup required. Handling this kind of cleanup in common code is more reliable because it doesn't require every target-arch maintainer to remember it needs to be done, plus it's less code. thanks -- PMM
Re: [PATCH V8 6/8] physmem: Add helper function to destroy CPU AddressSpace
On Mon, 6 May 2024 at 10:06, Salil Mehta wrote: > > Hi Peter, > > Thanks for the review. > > > From: Peter Maydell > > When do we need to destroy a single address space in this way that means > > we need to keep a count of how many ASes the CPU currently has? The > > commit message talks about the case when we unrealize the whole CPU > > object, but in that situation you can just throw away all the ASes at once > > (eg > > by calling some > > cpu_destroy_address_spaces() function from cpu_common_unrealizefn()). > > > Yes, maybe, we can destroy all at once from common leg as well. I'd prefer > this > to be done from the arch specific function for ARM to maintain the clarity & > symmetry of initialization and un-initialization legs. For now, all of these > address > space destruction is happening in context to the arm_cpu_unrealizefn(). > > It’s a kind of trade-off between little more code and clarity but I'm open to > further suggestions. > > > > > > Also, if we're leaking stuff here by failing to destroy it, is that a > > problem for > > existing CPU types like x86 that we can already hotplug? > > No we are not. We are taking care of these in the ARM arch specific legs > within functions arm_cpu_(un)realizefn(). How can you be taking care of *x86* CPU types in the Arm unrealize? thanks -- PMM
Re: [PATCH V8 6/8] physmem: Add helper function to destroy CPU AddressSpace
On Tue, 12 Mar 2024 at 02:02, Salil Mehta wrote:
>
> Virtual CPU Hot-unplug leads to unrealization of a CPU object. This also
> involves destruction of the CPU AddressSpace. Add common function to help
> destroy the CPU AddressSpace.
>
> Signed-off-by: Salil Mehta
> Tested-by: Vishnu Pajjuri
> Reviewed-by: Gavin Shan
> Tested-by: Xianglai Li
> Tested-by: Miguel Luis
> Reviewed-by: Shaoqin Huang
> diff --git a/system/physmem.c b/system/physmem.c
> index 6e9ed97597..61b32ac4f2 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -761,6 +761,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>
> if (!cpu->cpu_ases) {
> cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
> +cpu->cpu_ases_count = cpu->num_ases;
> }
>
> newas = >cpu_ases[asidx];
> @@ -774,6 +775,34 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
> }
> }
>
> +void cpu_address_space_destroy(CPUState *cpu, int asidx)
> +{
> +CPUAddressSpace *cpuas;
> +
> +assert(cpu->cpu_ases);
> +assert(asidx >= 0 && asidx < cpu->num_ases);
> +/* KVM cannot currently support multiple address spaces. */
> +assert(asidx == 0 || !kvm_enabled());
> +
> +cpuas = >cpu_ases[asidx];
> +if (tcg_enabled()) {
> +memory_listener_unregister(>tcg_as_listener);
> +}
> +
> +address_space_destroy(cpuas->as);
> +g_free_rcu(cpuas->as, rcu);
> +
> +if (asidx == 0) {
> +/* reset the convenience alias for address space 0 */
> +cpu->as = NULL;
> +}
> +
> +if (--cpu->cpu_ases_count == 0) {
> +g_free(cpu->cpu_ases);
> +cpu->cpu_ases = NULL;
> +}
> +}
When do we need to destroy a single address space in this way
that means we need to keep a count of how many ASes the CPU
currently has? The commit message talks about the case when we
unrealize the whole CPU object, but in that situation you can
just throw away all the ASes at once (eg by calling some
cpu_destroy_address_spaces() function from cpu_common_unrealizefn()).
Also, if we're leaking stuff here by failing to destroy it,
is that a problem for existing CPU types like x86 that we
can already hotplug?
thanks
-- PMM
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Fri, 3 May 2024 at 19:14, Dorjoy Chowdhury wrote:
>
> On Fri, May 3, 2024 at 10:28 PM Peter Maydell
> wrote:
> > In the meantime, there is one tiny bit of this that we can
> > do now:
> >
> > > diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> > > index cc68b5d8f1..9d5dcf1a3f 100644
> > > --- a/hw/arm/npcm7xx.c
> > > +++ b/hw/arm/npcm7xx.c
> > > @@ -487,7 +487,7 @@ static void npcm7xx_realize(DeviceState *dev, Error
> > > **errp)
> > > /* CPUs */
> > > for (i = 0; i < nc->num_cpus; i++) {
> > > object_property_set_int(OBJECT(>cpu[i]), "mp-affinity",
> > > -arm_build_mp_affinity(i,
> > > NPCM7XX_MAX_NUM_CPUS),
> > > +
> > > arm_build_mp_affinity(ARM_CPU(>cpu[i]), i, NPCM7XX_MAX_NUM_CPUS),
> > > _abort);
> > > object_property_set_int(OBJECT(>cpu[i]), "reset-cbar",
> > > NPCM7XX_GIC_CPU_IF_ADDR, _abort);
> >
> > In this file, the value of the mp-affinity property that the
> > board is setting is always the same as the default value it
> > would have anyway. So we can delete the call to
> > object_property_set_int() entirely, which gives us one fewer
> > place we need to deal with when we do eventually figure out
> > how the MPIDR values should work.
> >
>
> Before I send the patch removing the "object_property_set_int" line
> for "mp-affinity", just so that I understand, where else is it that
> for npcm7xx the mp_affinity is being set? I can't follow the code
> easily and I am not seeing where else it is being set to the same
> value. It's a bit hard to follow the initialization codes in QEMU.
The value that npcm7xx sets here is identical to the default value
that the Arm CPU will use if we don't set the property at all.
If the board doesn't set the property then the cpu mp_affinity field
is left at its default of ARM64_AFFINITY_INVALID, which then causes
arm_cpu_realizefn() to set it to the result of
arm_build_mp_affinity(cs->cpu_index, ARM_DEFAULT_CPUS_PER_CLUSTER)
Although ARM_DEFAULT_CPUS_PER_CLUSTER and NPCM7XX_MAX_NUM_CPUS are
different, the number of CPUs on an npcm7xx is always exactly 2,
so we never get to a CPU number high enough for that difference
to cause the mp_affinity value to be different from the default.
(The two CPUs get an mp_affinity of 0 and 1.)
thanks
-- PMM
Re: [PATCH v5] xlnx_dpdma: fix descriptor endianness bug
On Thu, 2 May 2024 at 15:16, Alexandra Diupina wrote:
>
> Add xlnx_dpdma_read_descriptor() and
> xlnx_dpdma_write_descriptor() functions.
> xlnx_dpdma_read_descriptor() combines reading a
> descriptor from desc_addr by calling dma_memory_read()
> and swapping the desc fields from guest memory order
> to host memory order. xlnx_dpdma_write_descriptor()
> performs similar actions when writing a descriptor.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d3c6369a96 ("introduce xlnx-dpdma")
> Signed-off-by: Alexandra Diupina
> @@ -755,8 +811,10 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s,
> uint8_t channel,
> /* The descriptor need to be updated when it's completed. */
> DPRINTF("update the descriptor with the done flag set.\n");
> xlnx_dpdma_desc_set_done();
> -dma_memory_write(_space_memory, desc_addr, ,
> - sizeof(DPDMADescriptor),
> MEMTXATTRS_UNSPECIFIED);
> +if (xlnx_dpdma_write_descriptor(desc_addr, )) {
> +DPRINTF("Can't write the descriptor.\n");
> +break;
> +}
This "break" introduces a behaviour change, so if we want it
it should not be in this patch, which is supposed to just
be fixing the endianness bug. (If we want to try to do the
right thing on write errors here we need to check the device
datasheet to see what it says about the hardware behaviour in
that situation.)
I dropped the "break" line and have queued this to target-arm.next.
thanks
-- PMM
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Fri, 19 Apr 2024 at 19:31, Dorjoy Chowdhury wrote:
>
> Some ARM CPUs advertise themselves as SMT by having the MT[24] bit set
> to 1 in the MPIDR register. These CPUs have the thread id in Aff0[7:0]
> bits, CPU id in Aff1[15:8] bits and cluster id in Aff2[23:16] bits in
> MPIDR.
>
> On the other hand, ARM CPUs without SMT have the MT[24] bit set to 0,
> CPU id in Aff0[7:0] bits and cluster id in Aff1[15:8] bits in MPIDR.
>
> The mpidr_read_val() function always reported non-SMT i.e., MT=0 style
> MPIDR value which means it was wrong for the following CPUs with SMT
> supported by QEMU:
> - cortex-a55
> - cortex-a76
> - cortex-a710
> - neoverse-v1
> - neoverse-n1
> - neoverse-n2
This has definitely turned out to be rather more complicated
than I thought it would be when I wrote up the original issue
in gitlab, so sorry about that.
I still need to think through how we should deal with the
interaction between what the CPU type implies about the MPIDR
format and the topology information provided by the user.
I probably won't get to that next week, because I'm on holiday
for most of it, but I will see if I can at least make a start.
In the meantime, there is one tiny bit of this that we can
do now:
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> index cc68b5d8f1..9d5dcf1a3f 100644
> --- a/hw/arm/npcm7xx.c
> +++ b/hw/arm/npcm7xx.c
> @@ -487,7 +487,7 @@ static void npcm7xx_realize(DeviceState *dev, Error
> **errp)
> /* CPUs */
> for (i = 0; i < nc->num_cpus; i++) {
> object_property_set_int(OBJECT(>cpu[i]), "mp-affinity",
> -arm_build_mp_affinity(i,
> NPCM7XX_MAX_NUM_CPUS),
> +arm_build_mp_affinity(ARM_CPU(>cpu[i]),
> i, NPCM7XX_MAX_NUM_CPUS),
> _abort);
> object_property_set_int(OBJECT(>cpu[i]), "reset-cbar",
> NPCM7XX_GIC_CPU_IF_ADDR, _abort);
In this file, the value of the mp-affinity property that the
board is setting is always the same as the default value it
would have anyway. So we can delete the call to
object_property_set_int() entirely, which gives us one fewer
place we need to deal with when we do eventually figure out
how the MPIDR values should work.
If you like you can submit a separate patch which deletes
this one call.
thanks
-- PMM
Re: [PULL 3/5] hw/loongarch: Add numa support
On Fri, 16 Jun 2023 at 11:03, Song Gao wrote:
>
> From: Tianrui Zhao
>
> 1. Implement some functions for LoongArch numa support;
> 2. Implement fdt_add_memory_node() for fdt;
> 3. build_srat() fills node_id and adds build numa memory.
>
> Reviewed-by: Song Gao
> Signed-off-by: Tianrui Zhao
> Signed-off-by: Song Gao
> Message-Id: <20230613122613.2471743-1-zhaotian...@loongson.cn>
Hi; Coverity has pointed out a memory leak in this commit
(CID 1544773):
> @@ -799,17 +823,43 @@ static void loongarch_init(MachineState *machine)
> machine->possible_cpus->cpus[i].cpu = OBJECT(cpu);
> }
> fdt_add_cpu_nodes(lams);
> -/* Add memory region */
> -memory_region_init_alias(>lowmem, NULL, "loongarch.lowram",
> - machine->ram, 0, 256 * MiB);
> -memory_region_add_subregion(address_space_mem, offset, >lowmem);
> -offset += 256 * MiB;
> -memmap_add_entry(0, 256 * MiB, 1);
> -highram_size = ram_size - 256 * MiB;
> -memory_region_init_alias(>highmem, NULL, "loongarch.highmem",
> - machine->ram, offset, highram_size);
> -memory_region_add_subregion(address_space_mem, 0x9000,
> >highmem);
> -memmap_add_entry(0x9000, highram_size, 1);
> +
> +/* Node0 memory */
> +memmap_add_entry(VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE, 1);
> +fdt_add_memory_node(machine, VIRT_LOWMEM_BASE, VIRT_LOWMEM_SIZE, 0);
> +memory_region_init_alias(>lowmem, NULL, "loongarch.node0.lowram",
> + machine->ram, offset, VIRT_LOWMEM_SIZE);
> +memory_region_add_subregion(address_space_mem, phyAddr, >lowmem);
> +
> +offset += VIRT_LOWMEM_SIZE;
> +if (nb_numa_nodes > 0) {
> +assert(numa_info[0].node_mem > VIRT_LOWMEM_SIZE);
> +highram_size = numa_info[0].node_mem - VIRT_LOWMEM_SIZE;
> +} else {
> +highram_size = ram_size - VIRT_LOWMEM_SIZE;
> +}
> +phyAddr = VIRT_HIGHMEM_BASE;
> +memmap_add_entry(phyAddr, highram_size, 1);
> +fdt_add_memory_node(machine, phyAddr, highram_size, 0);
> +memory_region_init_alias(>highmem, NULL, "loongarch.node0.highram",
> + machine->ram, offset, highram_size);
> +memory_region_add_subregion(address_space_mem, phyAddr, >highmem);
> +
> +/* Node1 - Nodemax memory */
> +offset += highram_size;
> +phyAddr += highram_size;
> +
> +for (i = 1; i < nb_numa_nodes; i++) {
> +MemoryRegion *nodemem = g_new(MemoryRegion, 1);
> +ramName = g_strdup_printf("loongarch.node%d.ram", i);
> +memory_region_init_alias(nodemem, NULL, ramName, machine->ram,
> + offset, numa_info[i].node_mem);
> +memory_region_add_subregion(address_space_mem, phyAddr, nodemem);
> +memmap_add_entry(phyAddr, numa_info[i].node_mem, 1);
> +fdt_add_memory_node(machine, phyAddr, numa_info[i].node_mem, i);
> +offset += numa_info[i].node_mem;
> +phyAddr += numa_info[i].node_mem;
In this loop, we allocate memory via g_strdup_printf(),
but never free it. The nicest fix for this is to use the
g_autofree mechanism so that the memory is automatically
freed when execution reaches the end of the block:
g_autofree ramName = g_strdup_printf("", ...);
thanks
-- PMM
Re: [PULL 04/53] hw/cxl: Add clear poison mailbox command support.
On Mon, 26 Jun 2023 at 13:28, Michael S. Tsirkin wrote:
>
> From: Jonathan Cameron
>
> Current implementation is very simple so many of the corner
> cases do not exist (e.g. fragmenting larger poison list entries)
Hi; Coverity has just spotted what looks like a bug in this
function (CID 1544772) where we write bogus data from the host
stack into guest memory):
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index ab600735eb..d751803188 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -947,6 +947,42 @@ static void set_lsa(CXLType3Dev *ct3d, const void *buf,
> uint64_t size,
> */
> }
>
> +static bool set_cacheline(CXLType3Dev *ct3d, uint64_t dpa_offset, uint8_t
> *data)
> +{
> +MemoryRegion *vmr = NULL, *pmr = NULL;
> +AddressSpace *as;
> +
> +if (ct3d->hostvmem) {
> +vmr = host_memory_backend_get_memory(ct3d->hostvmem);
> +}
> +if (ct3d->hostpmem) {
> +pmr = host_memory_backend_get_memory(ct3d->hostpmem);
> +}
> +
> +if (!vmr && !pmr) {
> +return false;
> +}
> +
> +if (dpa_offset + CXL_CACHE_LINE_SIZE > ct3d->cxl_dstate.mem_size) {
> +return false;
> +}
> +
> +if (vmr) {
> +if (dpa_offset < memory_region_size(vmr)) {
> +as = >hostvmem_as;
> +} else {
> +as = >hostpmem_as;
> +dpa_offset -= memory_region_size(vmr);
> +}
> +} else {
> +as = >hostpmem_as;
> +}
> +
> +address_space_write(as, dpa_offset, MEMTXATTRS_UNSPECIFIED, ,
> +CXL_CACHE_LINE_SIZE);
We've passed '' to address_space_write(), which means
"read from the address on the stack where the function
argument 'data' lives", so instead of writing 64 bytes of
data to the guest , we'll write 64 bytes which start with
a host pointer value and then continue with whatever happens
to be on the host stack after that.
I assume the intention was "data", not ""...
thanks
-- PMM
Re: [PULL v2 03/16] block/block-backend: add block layer APIs resembling Linux ZonedBlockDevice ioctls
On Mon, 15 May 2023 at 17:07, Stefan Hajnoczi wrote:
>
> From: Sam Li
>
> Add zoned device option to host_device BlockDriver. It will be presented only
> for zoned host block devices. By adding zone management operations to the
> host_block_device BlockDriver, users can use the new block layer APIs
> including Report Zone and four zone management operations
> (open, close, finish, reset, reset_all).
>
> Qemu-io uses the new APIs to perform zoned storage commands of the device:
> zone_report(zrp), zone_open(zo), zone_close(zc), zone_reset(zrs),
> zone_finish(zf).
>
> For example, to test zone_report, use following command:
> $ ./build/qemu-io --image-opts -n driver=host_device, filename=/dev/nullb0
> -c "zrp offset nr_zones"
Hi; Coverity points out an issue in this commit (CID 1544771):
> +static int zone_report_f(BlockBackend *blk, int argc, char **argv)
> +{
> +int ret;
> +int64_t offset;
> +unsigned int nr_zones;
> +
> +++optind;
> +offset = cvtnum(argv[optind]);
> +++optind;
> +nr_zones = cvtnum(argv[optind]);
cvtnum() can fail and return a negative value on error
(e.g. if the number in the string is out of range),
but we are not checking for that. Instead we stuff
the value into an 'unsigned int' and then pass that to
g_new(), which will result in our trying to allocate a large
amount of memory.
Here, and also in the other functions below that use cvtnum(),
I think we should follow the pattern for use of that function
that is used in the pre-existing code in this function:
int64_t foo; /* NB: not an unsigned or some smaller type */
foo = cvtnum(arg)
if (foo < 0) {
print_cvtnum_err(foo, arg);
return foo; /* or otherwise handle returning an error upward */
}
It looks like all the uses of cvtnum in this patch should be
adjusted to handle errors.
thanks
-- PMM
Re: [PATCH 00/14] hw: define and enforce a standard lifecycle for versioned machines
On Wed, 1 May 2024 at 19:28, Daniel P. Berrangé wrote: > I wonder, however, whether we would benefit from changing how we > update the VERSION file. > > eg instead of re-using the micro digit to indicate a dev or rc > snapshot, represent those explicitly. eg "9.1.0-dev" and > "9.1.0-rc1", "9.1.0-rc2", etc in VERSION. > > We don't use the full QEMU_VERSION in the code in all that many > places. It appears in some help messages for command line tools, > and in QMP query-version response, and in a few other misc places. > At a glance it appears all of those places would easily handle a > tagged version. > > For release candidates in particular I think it would be saner > to show the user the actual version the release is about to become, > rather than the previous release's version. This would make the > reported version match the rc tarball naming too which would be > nice. I think the theory behind the VERSION file is that we want to be able to express the version: * purely numerically * in a strictly ascending order We expose the assumption of numeric versions in places like QMP's query-version command, which reports it as a set of ints. I think there's probably scope for making the "human friendly" version string be surfaced instead of the strictly-numeric one in more places, but I worry that breaking the "always numeric and ascending" rule might have subtle breakage for us or for downstream uses... thanks -- PMM
Re: [PATCH] stm32l4x5_usart: add missing class_size
On Fri, 3 May 2024 at 12:10, Paolo Bonzini wrote:
>
> Depending on the phase of the moon, this seems to be causing CI failures on
> FreeBSD.
> Fortunately, valgrind catches it too, and in a fully deterministic way:
>
> ==210026== Invalid write of size 4
> ==210026==at 0x5222F3: stm32l4x5_lpuart_class_init (stm32l4x5_usart.c:611)
> ==210026==by 0xA499E1: object_class_foreach_tramp (object.c:1132)
> ==210026==by 0x5A60BEA: g_hash_table_foreach (ghash.c:2117)
> ==210026==by 0xA4A190: object_class_foreach (object.c:1154)
> ==210026==by 0xA4A190: object_class_get_list (object.c:1211)
> ==210026==by 0x7A5777: select_machine (vl.c:1664)
> ==210026==by 0x7A5777: qemu_create_machine (vl.c:2104)
> ==210026==by 0x7A5777: qemu_init (vl.c:3667)
> ==210026==by 0x47E528: main (main.c:47)
> ==210026== Address 0xe131340 is 0 bytes after a block of size 192 alloc'd
> ==210026==at 0x4849E60: calloc (vg_replace_malloc.c:1595)
> ==210026==by 0x5A79F71: g_malloc0 (gmem.c:133)
> ==210026==by 0xA48E9B: type_initialize (object.c:361)
> ==210026==by 0xA48E9B: type_initialize (object.c:336)
> ==210026==by 0xA499E1: object_class_foreach_tramp (object.c:1132)
> ==210026==by 0x5A60BEA: g_hash_table_foreach (ghash.c:2117)
> ==210026==by 0xA4A190: object_class_foreach (object.c:1154)
> ==210026==by 0xA4A190: object_class_get_list (object.c:1211)
> ==210026==by 0x7A5777: select_machine (vl.c:1664)
> ==210026==by 0x7A5777: qemu_create_machine (vl.c:2104)
> ==210026==by 0x7A5777: qemu_init (vl.c:3667)
> ==210026== by 0x47E528: main (main.c:47)
>
> Cc: Arnaud Minier
> Cc: Inès Varhol
> Cc: Peter Maydell
> Signed-off-by: Paolo Bonzini
> ---
> hw/char/stm32l4x5_usart.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
> index 2627aab8324..8dbcc7e19e7 100644
> --- a/hw/char/stm32l4x5_usart.c
> +++ b/hw/char/stm32l4x5_usart.c
> @@ -615,6 +615,7 @@ static const TypeInfo stm32l4x5_usart_types[] = {
> {
> .name = TYPE_STM32L4X5_USART_BASE,
> .parent = TYPE_SYS_BUS_DEVICE,
> +.class_size = sizeof(Stm32l4x5UsartBaseClass),
> .instance_size = sizeof(Stm32l4x5UsartBaseState),
> .instance_init = stm32l4x5_usart_base_init,
> .class_init = stm32l4x5_usart_base_class_init,
This is already upstream as commit afdc29b4a3a5, I think.
thanks
-- PMM
Re: QEMU headers in C++
On Thu, 2 May 2024 at 09:02, Daniel P. Berrangé wrote: > > On Wed, May 01, 2024 at 09:40:16PM -0700, Roman Kiryanov wrote: > > Hi QEMU, > > > > I work in Android Studio Emulator and we would like to develop devices > > in C++. Unfortunately, QEMU headers cannot be used with C++ as is > > (e.g. they use C++ keywords as variable names or implicitly cast void* > > to T*). > > NB, in recent past QEMU explicitly eliminated almost[1] all C++ code from > the tree, because the consensus was to be exlcusively a C project. > > > Will QEMU be open to accept patches from us to make QEMU headers C++ > > compatible? > > Personally I think that'd be a retrograde step. Any downstream development > fork that made use of that facility would be not be able to feed changes > / additions back into upstream QEMU codebase at a later date, without QEMU > accepting C++ code once again. > > We'll never control what forks can do, and many will never feed back code > regardless, but IMHO we should be steering external developers in a way > that keeps open the door for their changes to be merged back upstream. Yes, I agree. If you try to write device models in C++ then you're pretty heavily committing to never being able to upstream this stuff, plus you'll have a chunk of your codebase which is entirely "nobody else in the world does this this way". I really wouldn't recommend that. thanks -- PMM
[PATCH] ui/cocoa.m: Drop old macOS-10.12-and-earlier compat ifdefs
We only support the most recent two versions of macOS (currently macOS 13 Ventura and macOS 14 Sonoma), and our ui/cocoa.m code already assumes at least macOS 12 Monterey or better, because it uses NSScreen safeAreaInsets, which is 12.0-or-newer. Remove the ifdefs that were providing backwards compatibility for building on 10.12 and earlier versions. Signed-off-by: Peter Maydell --- ui/cocoa.m | 13 - 1 file changed, 13 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 25e0db9dd0b..981615a8b92 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -50,23 +50,10 @@ #include #include "hw/core/cpu.h" -#ifndef MAC_OS_X_VERSION_10_13 -#define MAC_OS_X_VERSION_10_13 101300 -#endif - #ifndef MAC_OS_VERSION_14_0 #define MAC_OS_VERSION_14_0 14 #endif -/* 10.14 deprecates NSOnState and NSOffState in favor of - * NSControlStateValueOn/Off, which were introduced in 10.13. - * Define for older versions - */ -#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_13 -#define NSControlStateValueOn NSOnState -#define NSControlStateValueOff NSOffState -#endif - //#define DEBUG #ifdef DEBUG -- 2.34.1
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Thu, 2 May 2024 at 14:50, Marcin Juszkiewicz wrote: > Both hw/arm/sbsa-ref.c and hw/arm/virt.c build cpu information in > DeviceTree using "arm_build_mp_afinnity()" function. So if firmware > parses it then it gets wrong values. What wrong values? The values in the dtb should match the Aff* fields, they are not the complete MPIDR_EL1 values including U and MT bits and so on: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/arm/cpus.yaml#n42 AIUI the ACPI spec says the same. > Firmware should probably read MPIDR_EL1 directly instead but what with > those who read DT and rely on this value already? Firmware should probably not read MPIDR_EL1 directly for topology information, because it's too vague and unreliable. Either it's real-hardware firmware, in which case it presumably knows the topology already, or else it's running on QEMU, in which case this is one of the things we can feed it via the DTB (either on virt or or sbsa-ref). thanks -- PMM
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Thu, 2 May 2024 at 14:11, Marcin Juszkiewicz wrote: > > W dniu 2.05.2024 o 15:04, Dorjoy Chowdhury pisze: > >> Should "return" also have "(1 << 24) |" to have MT=1 set? > >> > >> Otherwise MPIDR_EL1 = 0x000100 can mean core0 in cluster1 or core1 in > >> cluster0. > >> > >> Value 0x1000100 shows MT=1 so thread0 in core1 in cluster0. > > > I don't know all the details but from what I understand the > > "arm_build_mp_afiinity" is used to set the "mp_affinity" member > > variable which I assume is about affinity, not the whole MPIDR > > register value. That is what I assumed because the Uniprocessor > > indication bit(30) is being set only in the "mpidr_read_val" function. > > In the patch, the MT bit is also being set in the "mpidr_read_val" > > function based on the SMT status (has_smt) of the CPU. > > mpidr_read_val() is used only to set VMPIDR and VMPIDR_EL2 registers. > > So setting MT bit for MPIDR_EL1 needs to be added somewhere. The readfn for MPIDR_EL1 is mpidr_read(), which calls mpidr_read_val(). thanks -- PMM
Re: [PATCH] hw/display: Add SSD1306 dot matrix display controller support
On Tue, 30 Apr 2024 at 21:01, Ryan Mamone
wrote:
>
> From 617b2d92085d03524dcf5c223568a4856cdff47f Mon Sep 17 00:00:00 2001
>
> From: Ryan Mamone
>
> Date: Tue, 30 Apr 2024 13:20:50 -0400
>
> Subject: [PATCH] hw/display: Add SSD1306 dot matrix display controller support
Hi; thanks for this patch. It looks like unfortunately your
mail client has mangled it somewhat (among other things, it's
got sent in multipart/mixed with HTML, and for some reason when
I reply to it there are blank lines between every line). You
might want to look into sending it with a mail client that can
send plain text emails for the next version. (git send-email
is what I use.)
Anyway, some initial review comments (and some more interspersed
with the code below):
What's the purpose of this device model? Is it intended to be
used with a particular board model? What's an example of
a command line and/or guest setup demonstrating its use?
This is the kind of thing that's useful to provide in the
commit message, since it gives readers a rationale for why
this device is being added.
(Our existing ssd0303 and ssd0323 device models are both used
by boards we have models for: the stellaris boards.)
> +++ b/hw/display/ssd1306.c
>
> @@ -0,0 +1,612 @@
>
> +/*
>
> + * SSD1306 Dot Matrix Display Controller.
>
> + *
>
> + * The SSD1306 controller can support a variety of different displays up to
>
> + * 128 x 64. The dimensions of the emulated display can be configured by the
>
> + * 'width' and 'height' properties and has been tested using the most common
>
> + * displays dimensions of 128x64 and 128x32. A 'scaling' property has also
>
> + * been provided to perform integer pixel scaling of the output image to make
>
> + * it more viewable on pc displays. While the SSD1306 controller supports
>
> + * multiple physical interfaces this implementation only supports the I2C
>
> + * interface. Most of the commands relating to physical control, scrolling,
>
> + * multiplexing, and scanning direction are ignored.
>
> + *
>
> + * Copyright (C) 2024 Cambridge Consultants.
>
> + * Written by Ryan Mamone
>
> + *
>
> + * This code is licensed under the GPL.
This is ambiguous -- please can you specify the exact version
of the GPL you're licensing the code under, eg "GPLv2 or later"?
>
> + */
Is there a publicly available datasheet for the device?
The top of the file is a good place for a comment giving the
title of the datasheet and (if possible) a URL where it can
be obtained.
> +
>
> +#include "qemu/osdep.h"
>
> +#include "qemu/error-report.h"
>
> +#include "hw/i2c/i2c.h"
>
> +#include "hw/qdev-properties.h"
>
> +#include "migration/vmstate.h"
>
> +#include "qemu/module.h"
>
> +#include "ui/console.h"
>
> +#include "qom/object.h"
>
> +
>
> +/*#define DEBUG_SSD1306 1*/
>
> +
>
> +#ifdef DEBUG_SSD1306
>
> +#define DPRINTF(fmt, ...) \
>
> +do { error_printf("ssd1306: " fmt , ## __VA_ARGS__); } while (0)
>
> +#define BADF(fmt, ...) \
>
> +do { error_printf("ssd1306: error: " fmt , ## __VA_ARGS__); } while (0)
>
> +#else
>
> +#define DPRINTF(fmt, ...) do {} while (0)
>
> +#define BADF(fmt, ...) \
>
> +do { error_printf("ssd1306: error: " fmt , ## __VA_ARGS__); } while (0)
>
> +#endif
Please don't use DPRINTF debug printing in new code: prefer
tracepoints for debug type messages and qemu_log_mask() for
guest errors and not-yet-implemented messages.
>
> +
>
> +
>
> +/* Max supported display dimensions of the SSD1306 controller */
>
> +#define MAX_WIDTH 128
>
> +#define MAX_HEIGHT 64
>
> +/* Max supported color depth 32bit */
>
> +#define MAX_BPP 32
>
> +
>
> +enum ssd1306_addr_mode {
>
> +ssd1306_ADDR_MODE_HORIZ = 0,
>
> +ssd1306_ADDR_MODE_VERT = 1,
>
> +ssd1306_ADDR_MODE_PAGE = 2,
>
> +ssd1306_ADDR_MODE_INVALID
>
> +};
>
> +
>
> +enum ssd1306_mode {
>
> +ssd1306_IDLE,
>
> +ssd1306_DATA,
>
> +ssd1306_CMD,
>
> +ssd1306_CMD_DATA
>
> +};
>
> +
>
> +#define TYPE_SSD1306 "ssd1306"
>
> +OBJECT_DECLARE_SIMPLE_TYPE(ssd1306_state, SSD1306)
>
> +
>
> +struct ssd1306_state {
>
> +I2CSlave parent_obj;
>
> +
>
> +QemuConsole *con;
>
> +/* Emulated display dimensions */
>
> +uint8_t width;
>
> +uint8_t height;
>
> +/* Integer scaling factor to enlarge pixels for better viewing on PC
> displays */
>
> +uint8_t scaling_factor;
I don't think we should do this. AFAIK no other display device
does this; if we wanted to do it we'd probably be better off
doing it generically at the UI level.
>
> +uint8_t addr_mode;
>
> +uint8_t col;
>
> +uint8_t col_start;
>
> +uint8_t col_end;
>
> +uint8_t page;
>
> +uint8_t page_start;
>
> +uint8_t page_end;
>
> +int mirror;
>
> +int flash;
>
> +int enabled;
>
> +int inverse;
>
> +int redraw;
These seem to be used as booleans, so prefer "bool".
>
> +enum ssd1306_mode mode;
>
> +uint8_t cmd; /* Command ID byte */
>
> +uint8_t cmd_byte_num; /* Command data parameter number */
>
> +uint8_t
Re: [PATCH] hw/input/tsc2005: Fix -Wchar-subscripts warning in tsc2005_txrx()
On Thu, 2 May 2024 at 08:42, Philippe Mathieu-Daudé wrote:
>
> Check the function index is not negative and use an unsigned
> variable to avoid the following warning with GCC 13.2.0:
>
> [666/5358] Compiling C object libcommon.fa.p/hw_input_tsc2005.c.o
> hw/input/tsc2005.c: In function 'tsc2005_timer_tick':
> hw/input/tsc2005.c:416:26: warning: array subscript has type 'char'
> [-Wchar-subscripts]
> 416 | s->dav |= mode_regs[s->function];
> | ~^~
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/input/tsc2005.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
> index 941f163d36..fa93eb5d25 100644
> --- a/hw/input/tsc2005.c
> +++ b/hw/input/tsc2005.c
> @@ -406,6 +406,7 @@ uint32_t tsc2005_txrx(void *opaque, uint32_t value, int
> len)
> static void tsc2005_timer_tick(void *opaque)
> {
> TSC2005State *s = opaque;
> +unsigned func_idx;
>
> /* Timer ticked -- a set of conversions has been finished. */
>
> @@ -413,7 +414,9 @@ static void tsc2005_timer_tick(void *opaque)
> return;
>
> s->busy = false;
> -s->dav |= mode_regs[s->function];
> +assert(s->function >= 0);
> +func_idx = s->function;
> +s->dav |= mode_regs[func_idx];
> s->function = -1;
> tsc2005_pin_update(s);
> }
This code is in a deprecated device, but it makes sense to
avoid the compiler warning in the meantime. (I'm curious why this
only comes up now, since I thought this warning had been around
for a while. Maybe gcc 13 is applying it in a wider range of
situations.)
If we're going to assert I think we might as well catch
array overruns in both directions:
unsigned int function = s->function;
assert(function < ARRAY_SIZE(mode_regs);
s->dav |= mode_regs[function];
thanks
-- PMM
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Thu, 2 May 2024 at 11:56, Marcin Juszkiewicz wrote: > > W dniu 2.05.2024 o 12:37, Peter Maydell pisze: > >> * what are the constraints on the Aff* fields (eg that kernel > >> commit suggests Aff0 shouldn't be > 15)? > > > This one is apparently related to GICv3 -- if the GIC doesn't > > implement RangeSelector support in ICC_SGI0R_EL1 and other > > places (advertised via GICD_TYPER.RSS and ICC_CTLR_EL1.SS) then > > there's no way to send an SGI to a CPU whose Aff0 is outside > > [0..15], and so you shouldn't build a system with Aff0 > 15. > > QEMU's GICv3 doesn't implement the RSS functionality (though it > > wouldn't be hard to add if we really cared), so we should also > > keep Aff0 in [0..15]. > > Arm/virt uses 8 cores/cluster on GICv2 and 16 cores/cluster on GICv3 as > this is amount of SGI target-list bits available. You can't have more than 8 cores full stop on GICv2, though, so you'll never be able to go beyond 8 cores/cluster even if we didn't impose that limit. -- PMM
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Thu, 2 May 2024 at 10:11, Peter Maydell wrote: > On the QEMU side I guess we should strive to set up the MPIDR > fields to something plausibly matching the topology as defined > by the user on the command line. Unanswered questions: > > * I guess we need some kind of back-compat thing where for >old machine types we continue to report the old MPIDR > * what are the constraints on the Aff* fields (eg that kernel >commit suggests Aff0 shouldn't be > 15)? This one is apparently related to GICv3 -- if the GIC doesn't implement RangeSelector support in ICC_SGI0R_EL1 and other places (advertised via GICD_TYPER.RSS and ICC_CTLR_EL1.SS) then there's no way to send an SGI to a CPU whose Aff0 is outside [0..15], and so you shouldn't build a system with Aff0 > 15. QEMU's GICv3 doesn't implement the RSS functionality (though it wouldn't be hard to add if we really cared), so we should also keep Aff0 in [0..15]. We have ARM_DEFAULT_CPUS_PER_CLUSTER = 8, which does keep us in that range. I don't think there's really a good reason for it to be 8 rather than 16: this might be legacy from GICv2? -- PMM
Re: [PATCH v1 0/2] Upgrade ACPI SPCR table to support SPCR table version 4 format
On Thu, 2 May 2024 at 06:12, Sia Jee Heng wrote: > > Update the SPCR table to accommodate the SPCR Table version 4 [1]. > The SPCR table has been modified to adhere to the version 4 format [2]. > > Meanwhile, the virt SPCR golden reference files have been updated to > accommodate the SPCR Table version 4. > > [1]: > https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table > [2]: https://github.com/acpica/acpica/pull/931 > > Sia Jee Heng (2): > tests/qtest/bios-tables-test: Update virt SPCR golden references > hw/acpi: Upgrade ACPI SPCR table to support SPCR table version 4 > format This isn't the right way to make a change that requires updates to the bios-tables-test reference files, because "make check" will fail after patch 1 but before patch 2. You need a three-patch approach. How to do that is documented in the comment at the top of bios-tables-test.c. The resulting three patches should look like: * patch 1 updates bios-tables-test-allowed-diff.h to mark the affected test or tests as "OK to fail" * patch 2 makes the changes to QEMU that alter the required table output * patch 3 updates the reference files and removes the tests from the allowed-diff file See for instance commits 6c1c2e912fcf9, 1ec896fe7ca938, ea2fde5bccc514 as an example. Side note: if riscv virt has APCI tables now, maybe we should add testing of them to the bios-tables-test ? thanks -- PMM
Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT
On Wed, 1 May 2024 at 19:08, Marcin Juszkiewicz wrote: > > W dniu 22.04.2024 o 17:21, Richard Henderson pisze: > >>> For Arm's CPUs they fall into two categories: > >>> * older ones don't set MT in their MPIDR, and the Aff0 > >>> field is effectively the CPU number > >>> * newer ones do set MT in their MPIDR, but don't have > >>> SMT, so their Aff0 is always 0 and their Aff1 > >>> is the CPU number > >>> > >>> Of all the CPUs we model, none of them are the > >>> architecturally-permitted "MT is set, CPU implements > >>> actual SMT, Aff0 indicates the thread in the CPU" type. > >> > >> Looking at the TRM, Neoverse-E1 is "MT is set, actual SMT, > >> Aff0 is the thread" (Aff0 can be 0 or 1). We just don't > >> model that CPU type yet. But we should probably make > >> sure we don't block ourselves into a corner where that > >> would be awkward -- I'll have a think about this and > >> look at what x86 does with the topology info. > > > > I'm suggesting that we set things up per -smp, and if the user chooses a > > -cpu value for which that topology doesn't make sense, we do it anyway > > and let them keep both pieces. > > Aff[0-3] are 8 bit each. On those cpus where they exist. > > So "-smp 512" (maximum allowed for sbsa-ref) would need to be split to 2 > clusters by 256 cores or 64 clusters of 8 cores each like it is today so > it is backward compatible with whatever assumption firmware/OS does. > > But if we go for 'newer, better MPIDR_EL1' then maybe it is time to set > U bit [30] if "-smp X,sockets=Y" where Y > 1? Or when NUMA config with > multiple cpu nodes are setup. The U bit is a red herring here -- it indicates a property of the implementation, not of the topology. It is an almost obsolete bit that might have been useful on 32-bit cores (eg the A9 had a UP version and an MP version; we only implement the MP version, and the Cortex-R5 sets the U bit). I think any modern CPU will have U=0, whether it happens to be the only CPU in the system or not. > Also a way to know which AffX fields to check on firmware/OS side would > be nice. A57/72 use Aff[1-2], N1+ use Aff[0-3]. Sure, it can be checked > by going through cores, reading then MPIDR_EL1 and if 7:0 has same value > on all of them then check Aff[1-3], otherwise Aff[1-2]. I think the answer here is "guest software doesn't rely on the Aff fields to indicate topology". See eg this patch from back in 2020 to the kernel: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20200829130016.26106-1-valentin.schnei...@arm.com/ (now kernel commit 3102bc0e6ac752cc5) which notes that the amount of implementation leniency in the Aff* definitions and some of the odd stuff that has been shipped in practice means that you can't get useful topology information out of the MPIDR (which is why we provide it in the ACPI/dtb). On the QEMU side I guess we should strive to set up the MPIDR fields to something plausibly matching the topology as defined by the user on the command line. Unanswered questions: * I guess we need some kind of back-compat thing where for old machine types we continue to report the old MPIDR * what are the constraints on the Aff* fields (eg that kernel commit suggests Aff0 shouldn't be > 15)? * should user attempts to eg ask for a topology with threads > 1 but a CPU type which isn't multithreaded be an error? (What is the existing QEMU x86 practice here?) thanks -- PMM
Re: [PATCH 1/2] accel/tcg: Make TCGCPUOps::cpu_exec_halt return bool for whether to halt
On Tue, 30 Apr 2024 at 18:15, Alex Bennée wrote:
>
> Peter Maydell writes:
>
> > The TCGCPUOps::cpu_exec_halt method is called from cpu_handle_halt()
> > when the CPU is halted, so that a target CPU emulation can do
> > anything target-specific it needs to do. (At the moment we only use
> > this on i386.)
> >
> > The current specification of the method doesn't allow the target
> > specific code to do something different if the CPU is about to come
> > out of the halt state, because cpu_handle_halt() only determines this
> > after the method has returned. (If the method called cpu_has_work()
> > itself this would introduce a potential race if an interrupt arrived
> > between the target's method implementation checking and
> > cpu_handle_halt() repeating the check.)
> >
> > Change the definition of the method so that it returns a bool to
> > tell cpu_handle_halt() whether to stay in halt or not.
> >
> > We will want this for the Arm target, where FEAT_WFxT wants to do
> > some work only for the case where the CPU is in halt but about to
> > leave it.
> >
> > Signed-off-by: Peter Maydell
> > ---
> > include/hw/core/tcg-cpu-ops.h | 11 +--
> > target/i386/tcg/helper-tcg.h| 2 +-
> > accel/tcg/cpu-exec.c| 7 +--
> > target/i386/tcg/sysemu/seg_helper.c | 3 ++-
> > 4 files changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
> > index dc1f16a9777..f3ac76e6f6d 100644
> > --- a/include/hw/core/tcg-cpu-ops.h
> > +++ b/include/hw/core/tcg-cpu-ops.h
> > @@ -111,8 +111,15 @@ struct TCGCPUOps {
> > void (*do_interrupt)(CPUState *cpu);
> > /** @cpu_exec_interrupt: Callback for processing interrupts in
> > cpu_exec */
> > bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
> > -/** @cpu_exec_halt: Callback for handling halt in cpu_exec */
> > -void (*cpu_exec_halt)(CPUState *cpu);
> > +/**
> > + * @cpu_exec_halt: Callback for handling halt in cpu_exec.
> > + *
> > + * Return true to indicate that the CPU should now leave halt, false
> > + * if it should remain in the halted state.
> > + * If this method is not provided, the default is to leave halt
> > + * if cpu_has_work() returns true.
> > + */
> > +bool (*cpu_exec_halt)(CPUState *cpu);
>
> Would it be too much to rename the method to cpu_exec_leave_halt() to
> make it clearer on use the sense of the return value?
We could, but that makes it sound like it's a method to say
"should we leave halt?", which ...
> > -void x86_cpu_exec_halt(CPUState *cpu)
> > +bool x86_cpu_exec_halt(CPUState *cpu)
> > {
> > if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
> > X86CPU *x86_cpu = X86_CPU(cpu);
> > @@ -138,6 +138,7 @@ void x86_cpu_exec_halt(CPUState *cpu)
> > cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
> > bql_unlock();
> > }
> > +return cpu_has_work(cpu);
>
> The x86 version is essentially being called for side effects. Do we want
> to document this usage in the method?
...is not how the x86 target is using it, as you note.
thanks
-- PMM
Re: [PATCH 2/2] target/arm: Implement FEAT WFxT and enable for '-cpu max'
On Tue, 30 Apr 2024 at 18:31, Richard Henderson
wrote:
>
> On 4/30/24 07:00, Peter Maydell wrote:
> > +if (uadd64_overflow(timeout, offset, )) {
> > +nexttick = UINT64_MAX;
> > +}
> > +if (nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
> > +/*
> > + * If the timeout is too long for the signed 64-bit range
> > + * of a QEMUTimer, let it expire early.
> > + */
> > +timer_mod_ns(cpu->wfxt_timer, INT64_MAX);
> > +} else {
> > +timer_mod(cpu->wfxt_timer, nexttick);
> > +}
>
> The use of both UINT64_MAX and INT64_MAX is confusing. Perhaps
>
> if (uadd64_overflow(timeout, offset, ) ||
> nexttick > INT64_MAX / gt_cntfrq_period_ns(cpu)) {
> nexttick = INT64_MAX;
> }
> timer_mod(cpu->wfxt_timer, nexttick);
I'm following here the pattern of the logic in gt_recalc_timer()
(which could admittedly also be considered confusing...).
Also note that timer_mod_ns() and timer_mod() aren't the
same thing. The latter calls timer_mod_ns() on its argument
multiplied by ts->scale, so if you pass it INT64_MAX
the multiply is liable to overflow.
thanks
-- PMM
[PULL 11/21] hw/arm/sbsa-ref: Force CPU generic timer to 62.5MHz
Currently QEMU CPUs always run with a generic timer counter frequency
of 62.5MHz, but ARMv8.6 CPUs will run at 1GHz. For older versions of
the TF-A firmware that sbsa-ref runs, the frequency of the generic
timer is hardcoded into the firmware, and so if the CPU actually has
a different frequency then timers in the guest will be set
incorrectly.
The default frequency used by the 'max' CPU is about to change, so
make the sbsa-ref board force the CPU frequency to the value which
the firmware expects.
Newer versions of TF-A will read the frequency from the CPU's
CNTFRQ_EL0 register:
https://github.com/ARM-software/arm-trusted-firmware/commit/4c77fac98dac0bebc63798aae9101ac865b87148
so in the longer term we could make this board use the 1GHz
frequency. We will need to make sure we update the binaries used
by our avocado test
Aarch64SbsarefMachine.test_sbsaref_alpine_linux_max_pauth_impdef
before we can do that.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Marcin Juszkiewicz
Message-id: 20240426122913.3427983-3-peter.mayd...@linaro.org
---
hw/arm/sbsa-ref.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f5709d6c141..36f6f717b4b 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -60,6 +60,19 @@
#define NUM_SMMU_IRQS 4
#define NUM_SATA_PORTS 6
+/*
+ * Generic timer frequency in Hz (which drives both the CPU generic timers
+ * and the SBSA watchdog-timer). Older versions of the TF-A firmware
+ * typically used with sbsa-ref (including the binaries in our Avocado test
+ * Aarch64SbsarefMachine.test_sbsaref_alpine_linux_max_pauth_impdef
+ * assume it is this value.
+ *
+ * TODO: this value is not architecturally correct for an Armv8.6 or
+ * better CPU, so we should move to 1GHz once the TF-A fix above has
+ * made it into a release and into our Avocado test.
+ */
+#define SBSA_GTIMER_HZ 6250
+
enum {
SBSA_FLASH,
SBSA_MEM,
@@ -767,6 +780,8 @@ static void sbsa_ref_init(MachineState *machine)
_abort);
}
+object_property_set_int(cpuobj, "cntfrq", SBSA_GTIMER_HZ,
_abort);
+
object_property_set_link(cpuobj, "memory", OBJECT(sysmem),
_abort);
--
2.34.1
[PULL 13/21] target/arm: Default to 1GHz cntfrq for 'max' and new CPUs
In previous versions of the Arm architecture, the frequency of the
generic timers as reported in CNTFRQ_EL0 could be any IMPDEF value,
and for QEMU we picked 62.5MHz, giving a timer tick period of 16ns.
In Armv8.6, the architecture standardized this frequency to 1GHz.
Because there is no ID register feature field that indicates whether
a CPU is v8.6 or that it ought to have this counter frequency, we
implement this by changing our default CNTFRQ value for all CPUs,
with exceptions for backwards compatibility:
* CPU types which we already implement will retain the old
default value. None of these are v8.6 CPUs, so this is
architecturally OK.
* CPUs used in versioned machine types with a version of 9.0
or earlier will retain the old default value.
The upshot is that the only CPU type that changes is 'max'; but any
new type we add in future (whether v8.6 or not) will also get the new
1GHz default.
It remains the case that the machine model can override the default
value via the 'cntfrq' QOM property (regardless of the CPU type).
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240426122913.3427983-5-peter.mayd...@linaro.org
---
target/arm/cpu.h | 11 +++
target/arm/internals.h | 12 ++--
hw/core/machine.c | 4 +++-
target/arm/cpu.c | 23 +--
target/arm/cpu64.c | 2 ++
target/arm/tcg/cpu32.c | 4
target/arm/tcg/cpu64.c | 18 ++
7 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1f90590f937..a550bcd25fe 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -956,6 +956,9 @@ struct ArchCPU {
*/
bool host_cpu_probe_failed;
+/* QOM property to indicate we should use the back-compat CNTFRQ default */
+bool backcompat_cntfrq;
+
/* Specify the number of cores in this CPU cluster. Used for the L2CTLR
* register.
*/
@@ -2373,6 +2376,14 @@ enum arm_features {
ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
ARM_FEATURE_M_MAIN, /* M profile Main Extension */
ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
+/*
+ * ARM_FEATURE_BACKCOMPAT_CNTFRQ makes the CPU default cntfrq be 62.5MHz
+ * if the board doesn't set a value, instead of 1GHz. It is for backwards
+ * compatibility and used only with CPU definitions that were already
+ * in QEMU before we changed the default. It should not be set on any
+ * CPU types added in future.
+ */
+ARM_FEATURE_BACKCOMPAT_CNTFRQ, /* 62.5MHz timer default */
};
static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index b6c78db0243..ee3ebd383e1 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -62,9 +62,17 @@ static inline bool excp_is_internal(int excp)
/*
* Default frequency for the generic timer, in Hz.
- * This is 62.5MHz, which gives a 16 ns tick period.
+ * ARMv8.6 and later CPUs architecturally must use a 1GHz timer; before
+ * that it was an IMPDEF choice, and QEMU initially picked 62.5MHz,
+ * which gives a 16ns tick period.
+ *
+ * We will use the back-compat value:
+ * - for QEMU CPU types added before we standardized on 1GHz
+ * - for versioned machine types with a version of 9.0 or earlier
+ * In any case, the machine model may override via the cntfrq property.
*/
-#define GTIMER_DEFAULT_HZ 6250
+#define GTIMER_DEFAULT_HZ 10
+#define GTIMER_BACKCOMPAT_HZ 6250
/* Bit definitions for the v7M CONTROL register */
FIELD(V7M_CONTROL, NPRIV, 0, 1)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0dec48e8021..4ff60911e74 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -33,7 +33,9 @@
#include "hw/virtio/virtio-iommu.h"
#include "audio/audio.h"
-GlobalProperty hw_compat_9_0[] = {};
+GlobalProperty hw_compat_9_0[] = {
+{"arm-cpu", "backcompat-cntfrq", "true" },
+};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
GlobalProperty hw_compat_8_2[] = {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 9f2ca6633a1..fdc3eda318a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1959,13 +1959,22 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
if (!cpu->gt_cntfrq_hz) {
/*
- * 0 means "the board didn't set a value, use the default".
- * The default value of the generic timer frequency (as seen in
- * CNTFRQ_EL0) is 62.5MHz, which corresponds to a period of 16ns.
- * This is what you get (a) for a CONFIG_USER_ONLY CPU (b) if the
- * board doesn't set it.
+ * 0 means "the board didn't set a value, use the default". (We also
+ * get here for the CONFIG_USER_ONLY case.)
+ * ARMv8.6 and later CPUs architecturally must use a 1GHz timer; b
[PULL 10/21] target/arm: Refactor default generic timer frequency handling
The generic timer frequency is settable by board code via a QOM
property "cntfrq", but otherwise defaults to 62.5MHz. The way this
is done includes some complication resulting from how this was
originally a fixed value with no QOM property. Clean it up:
* always set cpu->gt_cntfrq_hz to some sensible value, whether
the CPU has the generic timer or not, and whether it's system
or user-only emulation
* this means we can always use gt_cntfrq_hz, and never need
the old GTIMER_SCALE define
* set the default value in exactly one place, in the realize fn
The aim here is to pave the way for handling the ARMv8.6 requirement
that the generic timer frequency is always 1GHz. We're going to do
that by having old CPU types keep their legacy-in-QEMU behaviour and
having the default for any new CPU types be a 1GHz rather han 62.5MHz
cntfrq, so we want the point where the default is decided to be in
one place, and in code, not in a DEFINE_PROP_UINT64() initializer.
This commit should have no behavioural changes.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-id: 20240426122913.3427983-2-peter.mayd...@linaro.org
---
target/arm/internals.h | 7 ---
target/arm/cpu.c | 31 +--
target/arm/helper.c| 16
3 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index e40ec453d56..b6c78db0243 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -60,10 +60,11 @@ static inline bool excp_is_internal(int excp)
|| excp == EXCP_SEMIHOST;
}
-/* Scale factor for generic timers, ie number of ns per tick.
- * This gives a 62.5MHz timer.
+/*
+ * Default frequency for the generic timer, in Hz.
+ * This is 62.5MHz, which gives a 16 ns tick period.
*/
-#define GTIMER_SCALE 16
+#define GTIMER_DEFAULT_HZ 6250
/* Bit definitions for the v7M CONTROL register */
FIELD(V7M_CONTROL, NPRIV, 0, 1)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a152def2413..9f2ca6633a1 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1506,9 +1506,12 @@ static void arm_cpu_initfn(Object *obj)
}
}
+/*
+ * 0 means "unset, use the default value". That default might vary depending
+ * on the CPU type, and is set in the realize fn.
+ */
static Property arm_cpu_gt_cntfrq_property =
-DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz,
- NANOSECONDS_PER_SECOND / GTIMER_SCALE);
+DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz, 0);
static Property arm_cpu_reset_cbar_property =
DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
@@ -1954,6 +1957,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
return;
}
+if (!cpu->gt_cntfrq_hz) {
+/*
+ * 0 means "the board didn't set a value, use the default".
+ * The default value of the generic timer frequency (as seen in
+ * CNTFRQ_EL0) is 62.5MHz, which corresponds to a period of 16ns.
+ * This is what you get (a) for a CONFIG_USER_ONLY CPU (b) if the
+ * board doesn't set it.
+ */
+cpu->gt_cntfrq_hz = GTIMER_DEFAULT_HZ;
+}
+
#ifndef CONFIG_USER_ONLY
/* The NVIC and M-profile CPU are two halves of a single piece of
* hardware; trying to use one without the other is a command line
@@ -2002,18 +2016,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
}
{
-uint64_t scale;
-
-if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
-if (!cpu->gt_cntfrq_hz) {
-error_setg(errp, "Invalid CNTFRQ: %"PRId64"Hz",
- cpu->gt_cntfrq_hz);
-return;
-}
-scale = gt_cntfrq_period_ns(cpu);
-} else {
-scale = GTIMER_SCALE;
-}
+uint64_t scale = gt_cntfrq_period_ns(cpu);
cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
arm_gt_ptimer_cb, cpu);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index bb0e1baf628..75876359608 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2474,6 +2474,13 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
.resetvalue = 0 },
};
+static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+cpu->env.cp15.c14_cntfrq = cpu->gt_cntfrq_hz;
+}
+
#ifndef CONFIG_USER_ONLY
static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo
*ri,
@@ -3228,13 +3235,6 @@ void arm_gt_hvtimer_cb(void *opaque)
gt_recalc_timer(cpu, GTIMER_HYPVIRT);
}
-static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
-{
-ARMCPU *cpu
[PULL 17/21] hw/display : Add device DM163
From: Inès Varhol
This device implements the IM120417002 colors shield v1.1 for Arduino
(which relies on the DM163 8x3-channel led driving logic) and features
a simple display of an 8x8 RGB matrix. The columns of the matrix are
driven by the DM163 and the rows are driven externally.
Acked-by: Alistair Francis
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240424200929.240921-2-ines.var...@telecom-paris.fr
[PMM: updated to new reset hold method prototype]
Signed-off-by: Peter Maydell
---
docs/system/arm/b-l475e-iot01a.rst | 3 +-
include/hw/display/dm163.h | 59 +
hw/display/dm163.c | 349 +
hw/display/Kconfig | 3 +
hw/display/meson.build | 1 +
hw/display/trace-events| 14 ++
6 files changed, 428 insertions(+), 1 deletion(-)
create mode 100644 include/hw/display/dm163.h
create mode 100644 hw/display/dm163.c
diff --git a/docs/system/arm/b-l475e-iot01a.rst
b/docs/system/arm/b-l475e-iot01a.rst
index a76c9976c50..2adcc4b4c16 100644
--- a/docs/system/arm/b-l475e-iot01a.rst
+++ b/docs/system/arm/b-l475e-iot01a.rst
@@ -12,7 +12,7 @@ USART, I2C, SPI, CAN and USB OTG, as well as a variety of
sensors.
Supported devices
"""""""""""""""""
-Currently B-L475E-IOT01A machine's only supports the following devices:
+Currently B-L475E-IOT01A machines support the following devices:
- Cortex-M4F based STM32L4x5 SoC
- STM32L4x5 EXTI (Extended interrupts and events controller)
@@ -20,6 +20,7 @@ Currently B-L475E-IOT01A machine's only supports the
following devices:
- STM32L4x5 RCC (Reset and clock control)
- STM32L4x5 GPIOs (General-purpose I/Os)
- STM32L4x5 USARTs, UARTs and LPUART (Serial ports)
+- optional 8x8 led display (based on DM163 driver)
Missing devices
"""""""""""""""
diff --git a/include/hw/display/dm163.h b/include/hw/display/dm163.h
new file mode 100644
index 000..4377f77bb75
--- /dev/null
+++ b/include/hw/display/dm163.h
@@ -0,0 +1,59 @@
+/*
+ * QEMU DM163 8x3-channel constant current led driver
+ * driving columns of associated 8x8 RGB matrix.
+ *
+ * Copyright (C) 2024 Samuel Tardieu
+ * Copyright (C) 2024 Arnaud Minier
+ * Copyright (C) 2024 Inès Varhol
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_DISPLAY_DM163_H
+#define HW_DISPLAY_DM163_H
+
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_DM163 "dm163"
+OBJECT_DECLARE_SIMPLE_TYPE(DM163State, DM163);
+
+#define RGB_MATRIX_NUM_ROWS 8
+#define RGB_MATRIX_NUM_COLS 8
+#define DM163_NUM_LEDS (RGB_MATRIX_NUM_COLS * 3)
+/* The last row is filled with 0 (turned off row) */
+#define COLOR_BUFFER_SIZE (RGB_MATRIX_NUM_ROWS + 1)
+
+typedef struct DM163State {
+DeviceState parent_obj;
+
+/* DM163 driver */
+uint64_t bank0_shift_register[3];
+uint64_t bank1_shift_register[3];
+uint16_t latched_outputs[DM163_NUM_LEDS];
+uint16_t outputs[DM163_NUM_LEDS];
+qemu_irq sout;
+
+uint8_t sin;
+uint8_t dck;
+uint8_t rst_b;
+uint8_t lat_b;
+uint8_t selbk;
+uint8_t en_b;
+
+/* IM120417002 colors shield */
+uint8_t activated_rows;
+
+/* 8x8 RGB matrix */
+QemuConsole *console;
+uint8_t redraw;
+/* Rows currently being displayed on the matrix. */
+/* The last row is filled with 0 (turned off row) */
+uint32_t buffer[COLOR_BUFFER_SIZE][RGB_MATRIX_NUM_COLS];
+uint8_t last_buffer_idx;
+uint8_t buffer_idx_of_row[RGB_MATRIX_NUM_ROWS];
+/* Used to simulate retinal persistence of rows */
+uint8_t row_persistence_delay[RGB_MATRIX_NUM_ROWS];
+} DM163State;
+
+#endif /* HW_DISPLAY_DM163_H */
diff --git a/hw/display/dm163.c b/hw/display/dm163.c
new file mode 100644
index 000..f92aee371d9
--- /dev/null
+++ b/hw/display/dm163.c
@@ -0,0 +1,349 @@
+/*
+ * QEMU DM163 8x3-channel constant current led driver
+ * driving columns of associated 8x8 RGB matrix.
+ *
+ * Copyright (C) 2024 Samuel Tardieu
+ * Copyright (C) 2024 Arnaud Minier
+ * Copyright (C) 2024 Inès Varhol
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * The reference used for the DM163 is the following :
+ * http://www.siti.com.tw/product/spec/LED/DM163.pdf
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/display/dm163.h"
+#include "ui/console.h"
+#include "trace.h"
+
+#define LED_SQUARE_SIZE 100
+/* Number of frames a row stays visible after being turned off. */
+#define ROW_PERSISTENCE 3
+#define TURNED_OFF_ROW (COLOR_BUFFER_SIZE - 1)
+
+static const VMStateDescriptio
[PULL 00/21] target-arm queue
Here's another arm pullreq; nothing too exciting in here I think. thanks -- PMM The following changes since commit 5fee33d97a7f2e95716417bd164f2f5264acd976: Merge tag 'samuel-thibault' of https://people.debian.org/~sthibault/qemu into staging (2024-04-29 14:34:25 -0700) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20240430 for you to fetch changes up to a0c325c4b05cf7815739d6a84e567b95c8c5be7e: tests/qtest : Add testcase for DM163 (2024-04-30 16:05:08 +0100) target-arm queue: * hw/core/clock: allow clock_propagate on child clocks * hvf: arm: Remove unused PL1_WRITE_MASK define * target/arm: Restrict translation disabled alignment check to VMSA * docs/system/arm/emulation.rst: Add missing implemented features * target/arm: Enable FEAT_CSV2_3, FEAT_ETS2, FEAT_Spec_FPACC for 'max' * tests/avocado: update sunxi kernel from armbian to 6.6.16 * target/arm: Make new CPUs default to 1GHz generic timer * hw/dmax/xlnx_dpdma: fix handling of address_extension descriptor fields * hw/char/stm32l4x5_usart: Fix memory corruption by adding correct class_size * hw/arm/npcm7xx: Store derivative OTP fuse key in little endian * hw/arm: Add DM163 display to B-L475E-IOT01A board Alexandra Diupina (1): hw/dmax/xlnx_dpdma: fix handling of address_extension descriptor fields Inès Varhol (5): hw/display : Add device DM163 hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC hw/arm : Create Bl475eMachineState hw/arm : Connect DM163 to B-L475E-IOT01A tests/qtest : Add testcase for DM163 Peter Maydell (10): docs/system/arm/emulation.rst: Add missing implemented features target/arm: Enable FEAT_CSV2_3 for -cpu max target/arm: Enable FEAT_ETS2 for -cpu max target/arm: Implement ID_AA64MMFR3_EL1 target/arm: Enable FEAT_Spec_FPACC for -cpu max tests/avocado: update sunxi kernel from armbian to 6.6.16 target/arm: Refactor default generic timer frequency handling hw/arm/sbsa-ref: Force CPU generic timer to 62.5MHz hw/watchdog/sbsa_gwdt: Make watchdog timer frequency a QOM property target/arm: Default to 1GHz cntfrq for 'max' and new CPUs Philippe Mathieu-Daudé (1): hw/arm/npcm7xx: Store derivative OTP fuse key in little endian Raphael Poggi (1): hw/core/clock: allow clock_propagate on child clocks Richard Henderson (1): target/arm: Restrict translation disabled alignment check to VMSA Thomas Huth (1): hw/char/stm32l4x5_usart: Fix memory corruption by adding correct class_size Zenghui Yu (1): hvf: arm: Remove PL1_WRITE_MASK docs/system/arm/b-l475e-iot01a.rst | 3 +- docs/system/arm/emulation.rst | 42 - include/hw/display/dm163.h | 59 ++ include/hw/watchdog/sbsa_gwdt.h | 3 +- target/arm/cpu.h| 28 +++ target/arm/internals.h | 15 +- hw/arm/b-l475e-iot01a.c | 105 +-- hw/arm/npcm7xx.c| 3 +- hw/arm/sbsa-ref.c | 16 ++ hw/arm/stm32l4x5_soc.c | 6 +- hw/char/stm32l4x5_usart.c | 1 + hw/core/clock.c | 1 - hw/core/machine.c | 4 +- hw/display/dm163.c | 349 hw/dma/xlnx_dpdma.c | 20 +-- hw/watchdog/sbsa_gwdt.c | 15 +- target/arm/cpu.c| 42 +++-- target/arm/cpu64.c | 2 + target/arm/helper.c | 22 +-- target/arm/hvf/hvf.c| 3 +- target/arm/kvm.c| 2 + target/arm/tcg/cpu32.c | 6 +- target/arm/tcg/cpu64.c | 28 ++- target/arm/tcg/hflags.c | 12 +- tests/qtest/dm163-test.c| 194 tests/qtest/stm32l4x5_gpio-test.c | 13 +- tests/qtest/stm32l4x5_syscfg-test.c | 17 +- hw/arm/Kconfig | 1 + hw/display/Kconfig | 3 + hw/display/meson.build | 1 + hw/display/trace-events | 14 ++ tests/avocado/boot_linux_console.py | 70 tests/avocado/replay_kernel.py | 8 +- tests/qtest/meson.build | 2 + 34 files changed, 987 insertions(+), 123 deletions(-) create mode 100644 include/hw/display/dm163.h create mode 100644 hw/display/dm163.c create mode 100644 tests/qtest/dm163-test.c
[PULL 01/21] hw/core/clock: allow clock_propagate on child clocks
From: Raphael Poggi
clock_propagate() has an assert that clk->source is NULL, i.e. that
you are calling it on a clock which has no source clock. This made
sense in the original design where the only way for a clock's
frequency to change if it had a source clock was when that source
clock changed. However, we subsequently added multiplier/divider
support, but didn't look at what that meant for propagation.
If a clock-management device changes the multiplier or divider value
on a clock, it needs to propagate that change down to child clocks,
even if the clock has a source clock set. So the assertion is now
incorrect.
Remove the assertion.
Signed-off-by: Raphael Poggi
Message-id: 20240419162951.23558-1-raphael.po...@lynxleap.co.uk
Reviewed-by: Peter Maydell
[PMM: Rewrote the commit message]
Signed-off-by: Peter Maydell
---
hw/core/clock.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/core/clock.c b/hw/core/clock.c
index a19c7db7df9..e212865307b 100644
--- a/hw/core/clock.c
+++ b/hw/core/clock.c
@@ -108,7 +108,6 @@ static void clock_propagate_period(Clock *clk, bool
call_callbacks)
void clock_propagate(Clock *clk)
{
-assert(clk->source == NULL);
trace_clock_propagate(CLOCK_PATH(clk));
clock_propagate_period(clk, true);
}
--
2.34.1
[PULL 04/21] docs/system/arm/emulation.rst: Add missing implemented features
As of version DDI0487K.a of the Arm ARM, some architectural features which previously didn't have official names have been named. Add these to the list of features which QEMU's TCG emulation supports. Mostly these are features which we thought of as part of baseline 8.0 support. For SVE and SVE2, the names have been brought into line with the FEAT_* naming convention of other extensions, and some sub-components split into separate FEAT_ items. In a few cases (eg FEAT_CCIDX, FEAT_DPB2) the omission from our list was just an oversight. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 20240418152004.2106516-2-peter.mayd...@linaro.org --- docs/system/arm/emulation.rst | 38 +-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index a9ae7ede9fc..5fdc64a944f 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -8,13 +8,26 @@ Armv8 versions of the A-profile architecture. It also has support for the following architecture extensions: - FEAT_AA32BF16 (AArch32 BFloat16 instructions) +- FEAT_AA32EL0 (Support for AArch32 at EL0) +- FEAT_AA32EL1 (Support for AArch32 at EL1) +- FEAT_AA32EL2 (Support for AArch32 at EL2) +- FEAT_AA32EL3 (Support for AArch32 at EL3) - FEAT_AA32HPD (AArch32 hierarchical permission disables) - FEAT_AA32I8MM (AArch32 Int8 matrix multiplication instructions) +- FEAT_AA64EL0 (Support for AArch64 at EL0) +- FEAT_AA64EL1 (Support for AArch64 at EL1) +- FEAT_AA64EL2 (Support for AArch64 at EL2) +- FEAT_AA64EL3 (Support for AArch64 at EL3) +- FEAT_AdvSIMD (Advanced SIMD Extension) - FEAT_AES (AESD and AESE instructions) +- FEAT_Armv9_Crypto (Armv9 Cryptographic Extension) +- FEAT_ASID16 (16 bit ASID) - FEAT_BBM at level 2 (Translation table break-before-make levels) - FEAT_BF16 (AArch64 BFloat16 instructions) - FEAT_BTI (Branch Target Identification) +- FEAT_CCIDX (Extended cache index) - FEAT_CRC32 (CRC32 instructions) +- FEAT_Crypto (Cryptographic Extension) - FEAT_CSV2 (Cache speculation variant 2) - FEAT_CSV2_1p1 (Cache speculation variant 2, version 1.1) - FEAT_CSV2_1p2 (Cache speculation variant 2, version 1.2) @@ -23,18 +36,27 @@ the following architecture extensions: - FEAT_DGH (Data gathering hint) - FEAT_DIT (Data Independent Timing instructions) - FEAT_DPB (DC CVAP instruction) +- FEAT_DPB2 (DC CVADP instruction) +- FEAT_Debugv8p1 (Debug with VHE) - FEAT_Debugv8p2 (Debug changes for v8.2) - FEAT_Debugv8p4 (Debug changes for v8.4) - FEAT_DotProd (Advanced SIMD dot product instructions) - FEAT_DoubleFault (Double Fault Extension) - FEAT_E0PD (Preventing EL0 access to halves of address maps) - FEAT_ECV (Enhanced Counter Virtualization) +- FEAT_EL0 (Support for execution at EL0) +- FEAT_EL1 (Support for execution at EL1) +- FEAT_EL2 (Support for execution at EL2) +- FEAT_EL3 (Support for execution at EL3) - FEAT_EPAC (Enhanced pointer authentication) - FEAT_ETS (Enhanced Translation Synchronization) - FEAT_EVT (Enhanced Virtualization Traps) +- FEAT_F32MM (Single-precision Matrix Multiplication) +- FEAT_F64MM (Double-precision Matrix Multiplication) - FEAT_FCMA (Floating-point complex number instructions) - FEAT_FGT (Fine-Grained Traps) - FEAT_FHM (Floating-point half-precision multiplication instructions) +- FEAT_FP (Floating Point extensions) - FEAT_FP16 (Half-precision floating-point data processing) - FEAT_FPAC (Faulting on AUT* instructions) - FEAT_FPACCOMBINE (Faulting on combined pointer authentication instructions) @@ -60,10 +82,13 @@ the following architecture extensions: - FEAT_LSE (Large System Extensions) - FEAT_LSE2 (Large System Extensions v2) - FEAT_LVA (Large Virtual Address space) +- FEAT_MixedEnd (Mixed-endian support) +- FEAT_MixdEndEL0 (Mixed-endian support at EL0) - FEAT_MOPS (Standardization of memory operations) - FEAT_MTE (Memory Tagging Extension) - FEAT_MTE2 (Memory Tagging Extension) - FEAT_MTE3 (MTE Asymmetric Fault Handling) +- FEAT_MTE_ASYM_FAULT (Memory tagging asymmetric faults) - FEAT_NMI (Non-maskable Interrupt) - FEAT_NV (Nested Virtualization) - FEAT_NV2 (Enhanced nested virtualization support) @@ -76,6 +101,7 @@ the following architecture extensions: - FEAT_PAuth (Pointer authentication) - FEAT_PAuth2 (Enhancements to pointer authentication) - FEAT_PMULL (PMULL, PMULL2 instructions) +- FEAT_PMUv3 (PMU extension version 3) - FEAT_PMUv3p1 (PMU Extensions v3.1) - FEAT_PMUv3p4 (PMU Extensions v3.4) - FEAT_PMUv3p5 (PMU Extensions v3.5) @@ -97,8 +123,18 @@ the following architecture extensions: - FEAT_SME_FA64 (Full A64 instruction set in Streaming SVE mode) - FEAT_SME_F64F64 (Double-precision floating-point outer product instructions) - FEAT_SME_I16I64 (16-bit to 64-bit integer widening outer product instructions) +- FEAT_SVE (Scalable Vector Extension) +- FEAT_SVE_AES (Scalable Vector AES instructions) +- FEAT_SVE_BitPerm (Scalable Vector Bit
[PULL 03/21] target/arm: Restrict translation disabled alignment check to VMSA
From: Richard Henderson
For cpus using PMSA, when the MPU is disabled, the default memory
type is Normal, Non-cachable. This means that it should not
have alignment restrictions enforced.
Cc: qemu-sta...@nongnu.org
Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when
translation disabled")
Reported-by: Clément Chigot
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Clément Chigot
Message-id: 20240422170722.117409-1-richard.hender...@linaro.org
[PMM: trivial comment, commit message tweaks]
Signed-off-by: Peter Maydell
---
target/arm/tcg/hflags.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c
index 5da1b0fc1d4..f03977b4b00 100644
--- a/target/arm/tcg/hflags.c
+++ b/target/arm/tcg/hflags.c
@@ -38,8 +38,16 @@ static bool aprofile_require_alignment(CPUARMState *env, int
el, uint64_t sctlr)
}
/*
- * If translation is disabled, then the default memory type is
- * Device(-nGnRnE) instead of Normal, which requires that alignment
+ * With PMSA, when the MPU is disabled, all memory types in the
+ * default map are Normal, so don't need aligment enforcing.
+ */
+if (arm_feature(env, ARM_FEATURE_PMSA)) {
+return false;
+}
+
+/*
+ * With VMSA, if translation is disabled, then the default memory type
+ * is Device(-nGnRnE) instead of Normal, which requires that alignment
* be enforced. Since this affects all ram, it is most efficient
* to handle this during translation.
*/
--
2.34.1
[PULL 20/21] hw/arm : Connect DM163 to B-L475E-IOT01A
From: Inès Varhol
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240424200929.240921-5-ines.var...@telecom-paris.fr
Signed-off-by: Peter Maydell
---
hw/arm/b-l475e-iot01a.c | 59 +++--
hw/arm/Kconfig | 1 +
2 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
index 970c637ce61..5002a40f06d 100644
--- a/hw/arm/b-l475e-iot01a.c
+++ b/hw/arm/b-l475e-iot01a.c
@@ -27,10 +27,37 @@
#include "hw/boards.h"
#include "hw/qdev-properties.h"
#include "qemu/error-report.h"
-#include "hw/arm/stm32l4x5_soc.h"
#include "hw/arm/boot.h"
+#include "hw/core/split-irq.h"
+#include "hw/arm/stm32l4x5_soc.h"
+#include "hw/gpio/stm32l4x5_gpio.h"
+#include "hw/display/dm163.h"
-/* B-L475E-IOT01A implementation is derived from netduinoplus2 */
+/* B-L475E-IOT01A implementation is inspired from netduinoplus2 and arduino */
+
+/*
+ * There are actually 14 input pins in the DM163 device.
+ * Here the DM163 input pin EN isn't connected to the STM32L4x5
+ * GPIOs as the IM120417002 colors shield doesn't actually use
+ * this pin to drive the RGB matrix.
+ */
+#define NUM_DM163_INPUTS 13
+
+static const unsigned dm163_input[NUM_DM163_INPUTS] = {
+1 * GPIO_NUM_PINS + 2, /* ROW0 PB2 */
+0 * GPIO_NUM_PINS + 15, /* ROW1 PA15 */
+0 * GPIO_NUM_PINS + 2, /* ROW2 PA2 */
+0 * GPIO_NUM_PINS + 7, /* ROW3 PA7 */
+0 * GPIO_NUM_PINS + 6, /* ROW4 PA6 */
+0 * GPIO_NUM_PINS + 5, /* ROW5 PA5 */
+1 * GPIO_NUM_PINS + 0, /* ROW6 PB0 */
+0 * GPIO_NUM_PINS + 3, /* ROW7 PA3 */
+0 * GPIO_NUM_PINS + 4, /* SIN (SDA) PA4 */
+1 * GPIO_NUM_PINS + 1, /* DCK (SCK) PB1 */
+2 * GPIO_NUM_PINS + 3, /* RST_B (RST) PC3 */
+2 * GPIO_NUM_PINS + 4, /* LAT_B (LAT) PC4 */
+2 * GPIO_NUM_PINS + 5, /* SELBK (SB) PC5 */
+};
#define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
@@ -39,12 +66,16 @@ typedef struct Bl475eMachineState {
MachineState parent_obj;
Stm32l4x5SocState soc;
+SplitIRQ gpio_splitters[NUM_DM163_INPUTS];
+DM163State dm163;
} Bl475eMachineState;
static void bl475e_init(MachineState *machine)
{
Bl475eMachineState *s = B_L475E_IOT01A(machine);
const Stm32l4x5SocClass *sc;
+DeviceState *dev, *gpio_out_splitter;
+unsigned gpio, pin;
object_initialize_child(OBJECT(machine), "soc", >soc,
TYPE_STM32L4X5XG_SOC);
@@ -53,6 +84,30 @@ static void bl475e_init(MachineState *machine)
sc = STM32L4X5_SOC_GET_CLASS(>soc);
armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0,
sc->flash_size);
+
+if (object_class_by_name(TYPE_DM163)) {
+object_initialize_child(OBJECT(machine), "dm163",
+>dm163, TYPE_DM163);
+dev = DEVICE(>dm163);
+qdev_realize(dev, NULL, _abort);
+
+for (unsigned i = 0; i < NUM_DM163_INPUTS; i++) {
+object_initialize_child(OBJECT(machine), "gpio-out-splitters[*]",
+>gpio_splitters[i], TYPE_SPLIT_IRQ);
+gpio_out_splitter = DEVICE(>gpio_splitters[i]);
+qdev_prop_set_uint32(gpio_out_splitter, "num-lines", 2);
+qdev_realize(gpio_out_splitter, NULL, _fatal);
+
+qdev_connect_gpio_out(gpio_out_splitter, 0,
+qdev_get_gpio_in(DEVICE(>soc), dm163_input[i]));
+qdev_connect_gpio_out(gpio_out_splitter, 1,
+qdev_get_gpio_in(dev, i));
+gpio = dm163_input[i] / GPIO_NUM_PINS;
+pin = dm163_input[i] % GPIO_NUM_PINS;
+qdev_connect_gpio_out(DEVICE(>soc.gpio[gpio]), pin,
+qdev_get_gpio_in(DEVICE(gpio_out_splitter), 0));
+}
+}
}
static void bl475e_machine_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e8b6e5e5ebc..fe1f9643bd9 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -468,6 +468,7 @@ config B_L475E_IOT01A
default y
depends on TCG && ARM
select STM32L4X5_SOC
+imply DM163
config STM32L4X5_SOC
bool
--
2.34.1
[PULL 14/21] hw/dmax/xlnx_dpdma: fix handling of address_extension descriptor fields
From: Alexandra Diupina
The DMA descriptor structures for this device have
a set of "address extension" fields which extend the 32
bit source addresses with an extra 16 bits to give a
48 bit address:
https://docs.amd.com/r/en-US/ug1085-zynq-ultrascale-trm/ADDR_EXT-Field
However, we misimplemented this address extension in several ways:
* we only extracted 12 bits of the extension fields, not 16
* we didn't shift the extension field up far enough
* we accidentally did the shift as 32-bit arithmetic, which
meant that we would have an overflow instead of setting
bits [47:32] of the resulting 64-bit address
Add a type cast and use extract64() instead of extract32()
to avoid integer overflow on addition. Fix bit fields
extraction according to documentation.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Cc: qemu-sta...@nongnu.org
Fixes: d3c6369a96 ("introduce xlnx-dpdma")
Signed-off-by: Alexandra Diupina
Message-id: 20240428181131.23801-1-adiup...@astralinux.ru
[PMM: adjusted commit message]
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/dma/xlnx_dpdma.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
index 1f5cd64ed10..530717d1885 100644
--- a/hw/dma/xlnx_dpdma.c
+++ b/hw/dma/xlnx_dpdma.c
@@ -175,24 +175,24 @@ static uint64_t
xlnx_dpdma_desc_get_source_address(DPDMADescriptor *desc,
switch (frag) {
case 0:
-addr = desc->source_address
-+ (extract32(desc->address_extension, 16, 12) << 20);
+addr = (uint64_t)desc->source_address
++ (extract64(desc->address_extension, 16, 16) << 32);
break;
case 1:
-addr = desc->source_address2
-+ (extract32(desc->address_extension_23, 0, 12) << 8);
+addr = (uint64_t)desc->source_address2
++ (extract64(desc->address_extension_23, 0, 16) << 32);
break;
case 2:
-addr = desc->source_address3
-+ (extract32(desc->address_extension_23, 16, 12) << 20);
+addr = (uint64_t)desc->source_address3
++ (extract64(desc->address_extension_23, 16, 16) << 32);
break;
case 3:
-addr = desc->source_address4
-+ (extract32(desc->address_extension_45, 0, 12) << 8);
+addr = (uint64_t)desc->source_address4
++ (extract64(desc->address_extension_45, 0, 16) << 32);
break;
case 4:
-addr = desc->source_address5
-+ (extract32(desc->address_extension_45, 16, 12) << 20);
+addr = (uint64_t)desc->source_address5
++ (extract64(desc->address_extension_45, 16, 16) << 32);
break;
default:
addr = 0;
--
2.34.1
[PULL 21/21] tests/qtest : Add testcase for DM163
From: Inès Varhol
`test_dm163_bank()`
Checks that the pin "sout" of the DM163 led driver outputs the values
received on pin "sin" with the expected latency (depending on the bank).
`test_dm163_gpio_connection()`
Check that changes to relevant STM32L4x5 GPIO pins are propagated to the
DM163 device.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
Acked-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240424200929.240921-6-ines.var...@telecom-paris.fr
Signed-off-by: Peter Maydell
---
tests/qtest/dm163-test.c | 194 +++
tests/qtest/meson.build | 2 +
2 files changed, 196 insertions(+)
create mode 100644 tests/qtest/dm163-test.c
diff --git a/tests/qtest/dm163-test.c b/tests/qtest/dm163-test.c
new file mode 100644
index 000..3161c9208d8
--- /dev/null
+++ b/tests/qtest/dm163-test.c
@@ -0,0 +1,194 @@
+/*
+ * QTest testcase for DM163
+ *
+ * Copyright (C) 2024 Samuel Tardieu
+ * Copyright (C) 2024 Arnaud Minier
+ * Copyright (C) 2024 Inès Varhol
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+enum DM163_INPUTS {
+SIN = 8,
+DCK = 9,
+RST_B = 10,
+LAT_B = 11,
+SELBK = 12,
+EN_B = 13
+};
+
+#define DEVICE_NAME "/machine/dm163"
+#define GPIO_OUT(name, value) qtest_set_irq_in(qts, DEVICE_NAME, NULL, name,
\
+ value)
+#define GPIO_PULSE(name)
\
+ do {
\
+GPIO_OUT(name, 1);
\
+GPIO_OUT(name, 0);
\
+ } while (0)
+
+
+static void rise_gpio_pin_dck(QTestState *qts)
+{
+/* Configure output mode for pin PB1 */
+qtest_writel(qts, 0x48000400, 0xFEB7);
+/* Write 1 in ODR for PB1 */
+qtest_writel(qts, 0x48000414, 0x0002);
+}
+
+static void lower_gpio_pin_dck(QTestState *qts)
+{
+/* Configure output mode for pin PB1 */
+qtest_writel(qts, 0x48000400, 0xFEB7);
+/* Write 0 in ODR for PB1 */
+qtest_writel(qts, 0x48000414, 0x);
+}
+
+static void rise_gpio_pin_selbk(QTestState *qts)
+{
+/* Configure output mode for pin PC5 */
+qtest_writel(qts, 0x48000800, 0xF7FF);
+/* Write 1 in ODR for PC5 */
+qtest_writel(qts, 0x48000814, 0x0020);
+}
+
+static void lower_gpio_pin_selbk(QTestState *qts)
+{
+/* Configure output mode for pin PC5 */
+qtest_writel(qts, 0x48000800, 0xF7FF);
+/* Write 0 in ODR for PC5 */
+qtest_writel(qts, 0x48000814, 0x);
+}
+
+static void rise_gpio_pin_lat_b(QTestState *qts)
+{
+/* Configure output mode for pin PC4 */
+qtest_writel(qts, 0x48000800, 0xFDFF);
+/* Write 1 in ODR for PC4 */
+qtest_writel(qts, 0x48000814, 0x0010);
+}
+
+static void lower_gpio_pin_lat_b(QTestState *qts)
+{
+/* Configure output mode for pin PC4 */
+qtest_writel(qts, 0x48000800, 0xFDFF);
+/* Write 0 in ODR for PC4 */
+qtest_writel(qts, 0x48000814, 0x);
+}
+
+static void rise_gpio_pin_rst_b(QTestState *qts)
+{
+/* Configure output mode for pin PC3 */
+qtest_writel(qts, 0x48000800, 0xFF7F);
+/* Write 1 in ODR for PC3 */
+qtest_writel(qts, 0x48000814, 0x0008);
+}
+
+static void lower_gpio_pin_rst_b(QTestState *qts)
+{
+/* Configure output mode for pin PC3 */
+qtest_writel(qts, 0x48000800, 0xFF7F);
+/* Write 0 in ODR for PC3 */
+qtest_writel(qts, 0x48000814, 0x);
+}
+
+static void rise_gpio_pin_sin(QTestState *qts)
+{
+/* Configure output mode for pin PA4 */
+qtest_writel(qts, 0x4800, 0xFDFF);
+/* Write 1 in ODR for PA4 */
+qtest_writel(qts, 0x4814, 0x0010);
+}
+
+static void lower_gpio_pin_sin(QTestState *qts)
+{
+/* Configure output mode for pin PA4 */
+qtest_writel(qts, 0x4800, 0xFDFF);
+/* Write 0 in ODR for PA4 */
+qtest_writel(qts, 0x4814, 0x);
+}
+
+static void test_dm163_bank(const void *opaque)
+{
+const unsigned bank = (uintptr_t) opaque;
+const int width = bank ? 192 : 144;
+
+QTestState *qts = qtest_initf("-M b-l475e-iot01a");
+qtest_irq_intercept_out_named(qts, DEVICE_NAME, "sout");
+GPIO_OUT(RST_B, 1);
+GPIO_OUT(EN_B, 0);
+GPIO_OUT(DCK, 0);
+GPIO_OUT(SELBK, bank);
+GPIO_OUT(LAT_B, 1);
+
+/* Fill bank with zeroes */
+GPIO_OUT(SIN, 0);
+for (int i = 0; i < width; i++) {
+GPIO_PULSE(DCK);
+}
+/* Fill bank with ones, check that we get the previous zeroes */
+GPIO_OUT(SIN, 1);
+for (int i = 0; i < width; i++) {
+GPIO_PULSE(DCK);
+g_assert(!qtest_get_irq(qts, 0));
+}
+
+/* Pulse one more bit in the bank, check that we get a one */
+
[PULL 15/21] hw/char/stm32l4x5_usart: Fix memory corruption by adding correct class_size
From: Thomas Huth
"make check-qtest-aarch64" recently started failing on FreeBSD builds,
and valgrind on Linux also detected that there is something fishy with
the new stm32l4x5-usart: The code forgot to set the correct class_size
here, so the various class_init functions in this file wrote beyond
the allocated buffer when setting the subc->type field.
Fixes: 4fb37aea7e ("hw/char: Implement STM32L4x5 USART skeleton")
Signed-off-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240429075908.36302-1-th...@redhat.com
Signed-off-by: Peter Maydell
---
hw/char/stm32l4x5_usart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
index 2627aab8324..02f666308c0 100644
--- a/hw/char/stm32l4x5_usart.c
+++ b/hw/char/stm32l4x5_usart.c
@@ -617,6 +617,7 @@ static const TypeInfo stm32l4x5_usart_types[] = {
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(Stm32l4x5UsartBaseState),
.instance_init = stm32l4x5_usart_base_init,
+.class_size = sizeof(Stm32l4x5UsartBaseClass),
.class_init = stm32l4x5_usart_base_class_init,
.abstract = true,
}, {
--
2.34.1
[PULL 07/21] target/arm: Implement ID_AA64MMFR3_EL1
Newer versions of the Arm ARM (e.g. rev K.a) now define fields for
ID_AA64MMFR3_EL1. Implement this register, so that we can set the
fields if we need to. There's no behaviour change here since we
don't currently set the register value to non-zero.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240418152004.2106516-5-peter.mayd...@linaro.org
---
target/arm/cpu.h | 17 +
target/arm/helper.c | 6 --
target/arm/hvf/hvf.c | 2 ++
target/arm/kvm.c | 2 ++
4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 17efc5d565a..1f90590f937 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1011,6 +1011,7 @@ struct ArchCPU {
uint64_t id_aa64mmfr0;
uint64_t id_aa64mmfr1;
uint64_t id_aa64mmfr2;
+uint64_t id_aa64mmfr3;
uint64_t id_aa64dfr0;
uint64_t id_aa64dfr1;
uint64_t id_aa64zfr0;
@@ -2206,6 +2207,22 @@ FIELD(ID_AA64MMFR2, BBM, 52, 4)
FIELD(ID_AA64MMFR2, EVT, 56, 4)
FIELD(ID_AA64MMFR2, E0PD, 60, 4)
+FIELD(ID_AA64MMFR3, TCRX, 0, 4)
+FIELD(ID_AA64MMFR3, SCTLRX, 4, 4)
+FIELD(ID_AA64MMFR3, S1PIE, 8, 4)
+FIELD(ID_AA64MMFR3, S2PIE, 12, 4)
+FIELD(ID_AA64MMFR3, S1POE, 16, 4)
+FIELD(ID_AA64MMFR3, S2POE, 20, 4)
+FIELD(ID_AA64MMFR3, AIE, 24, 4)
+FIELD(ID_AA64MMFR3, MEC, 28, 4)
+FIELD(ID_AA64MMFR3, D128, 32, 4)
+FIELD(ID_AA64MMFR3, D128_2, 36, 4)
+FIELD(ID_AA64MMFR3, SNERR, 40, 4)
+FIELD(ID_AA64MMFR3, ANERR, 44, 4)
+FIELD(ID_AA64MMFR3, SDERR, 52, 4)
+FIELD(ID_AA64MMFR3, ADERR, 56, 4)
+FIELD(ID_AA64MMFR3, SPEC_FPACC, 60, 4)
+
FIELD(ID_AA64DFR0, DEBUGVER, 0, 4)
FIELD(ID_AA64DFR0, TRACEVER, 4, 4)
FIELD(ID_AA64DFR0, PMUVER, 8, 4)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6b224826fbb..bb0e1baf628 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9004,11 +9004,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.access = PL1_R, .type = ARM_CP_CONST,
.accessfn = access_aa64_tid3,
.resetvalue = cpu->isar.id_aa64mmfr2 },
-{ .name = "ID_AA64MMFR3_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+{ .name = "ID_AA64MMFR3_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 3,
.access = PL1_R, .type = ARM_CP_CONST,
.accessfn = access_aa64_tid3,
- .resetvalue = 0 },
+ .resetvalue = cpu->isar.id_aa64mmfr3 },
{ .name = "ID_AA64MMFR4_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 4,
.access = PL1_R, .type = ARM_CP_CONST,
@@ -9165,6 +9165,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.exported_bits = R_ID_AA64MMFR1_AFP_MASK },
{ .name = "ID_AA64MMFR2_EL1",
.exported_bits = R_ID_AA64MMFR2_AT_MASK },
+{ .name = "ID_AA64MMFR3_EL1",
+ .exported_bits = 0 },
{ .name = "ID_AA64MMFR*_EL1_RESERVED",
.is_glob = true },
{ .name = "ID_AA64DFR0_EL1",
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 8e942f89b35..08d0757438c 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -497,6 +497,7 @@ static struct hvf_sreg_match hvf_sreg_match[] = {
#endif
{ HV_SYS_REG_ID_AA64MMFR1_EL1, HVF_SYSREG(0, 7, 3, 0, 1) },
{ HV_SYS_REG_ID_AA64MMFR2_EL1, HVF_SYSREG(0, 7, 3, 0, 2) },
+/* Add ID_AA64MMFR3_EL1 here when HVF supports it */
{ HV_SYS_REG_MDSCR_EL1, HVF_SYSREG(0, 2, 2, 0, 2) },
{ HV_SYS_REG_SCTLR_EL1, HVF_SYSREG(1, 0, 3, 0, 0) },
@@ -855,6 +856,7 @@ static bool
hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
{ HV_SYS_REG_ID_AA64MMFR0_EL1, _isar.id_aa64mmfr0 },
{ HV_SYS_REG_ID_AA64MMFR1_EL1, _isar.id_aa64mmfr1 },
{ HV_SYS_REG_ID_AA64MMFR2_EL1, _isar.id_aa64mmfr2 },
+/* Add ID_AA64MMFR3_EL1 here when HVF supports it */
};
hv_vcpu_t fd;
hv_return_t r = HV_SUCCESS;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 21ebbf3b8f8..7cf5cf31dec 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -331,6 +331,8 @@ static bool
kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
ARM64_SYS_REG(3, 0, 0, 7, 1));
err |= read_sys_reg64(fdarray[2], >isar.id_aa64mmfr2,
ARM64_SYS_REG(3, 0, 0, 7, 2));
+err |= read_sys_reg64(fdarray[2], >isar.id_aa64mmfr3,
+ ARM64_SYS_REG(3, 0, 0, 7, 3));
/*
* Note that if AArch32 support is not present in the host,
--
2.34.1
[PULL 18/21] hw/arm : Pass STM32L4x5 SYSCFG gpios to STM32L4x5 SoC
From: Inès Varhol
Exposing SYSCFG inputs to the SoC is practical in order to wire the SoC
to the optional DM163 display from the board code (GPIOs outputs need
to be connected to both SYSCFG inputs and DM163 inputs).
STM32L4x5 SYSCFG in-irq interception needed to be changed accordingly.
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240424200929.240921-3-ines.var...@telecom-paris.fr
Signed-off-by: Peter Maydell
---
hw/arm/stm32l4x5_soc.c | 6 --
tests/qtest/stm32l4x5_gpio-test.c | 13 -
tests/qtest/stm32l4x5_syscfg-test.c | 17 ++---
3 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 39924822f3d..38f7a2d5d9f 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -1,8 +1,8 @@
/*
* STM32L4x5 SoC family
*
- * Copyright (c) 2023 Arnaud Minier
- * Copyright (c) 2023 Inès Varhol
+ * Copyright (c) 2023-2024 Arnaud Minier
+ * Copyright (c) 2023-2024 Inès Varhol
*
* SPDX-License-Identifier: GPL-2.0-or-later
*
@@ -250,6 +250,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
Error **errp)
}
}
+qdev_pass_gpios(DEVICE(>syscfg), dev_soc, NULL);
+
/* EXTI device */
busdev = SYS_BUS_DEVICE(>exti);
if (!sysbus_realize(busdev, errp)) {
diff --git a/tests/qtest/stm32l4x5_gpio-test.c
b/tests/qtest/stm32l4x5_gpio-test.c
index 0f6bda54d3c..72a78234066 100644
--- a/tests/qtest/stm32l4x5_gpio-test.c
+++ b/tests/qtest/stm32l4x5_gpio-test.c
@@ -43,6 +43,9 @@
#define OTYPER_PUSH_PULL 0
#define OTYPER_OPEN_DRAIN 1
+/* SoC forwards GPIOs to SysCfg */
+#define SYSCFG "/machine/soc"
+
const uint32_t moder_reset[NUM_GPIOS] = {
0xABFF,
0xFEBF,
@@ -284,7 +287,7 @@ static void test_gpio_output_mode(const void *data)
uint32_t gpio = test_gpio_addr(data);
unsigned int gpio_id = get_gpio_id(gpio);
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Set a bit in ODR and check nothing happens */
gpio_set_bit(gpio, ODR, pin, 1);
@@ -319,7 +322,7 @@ static void test_gpio_input_mode(const void *data)
uint32_t gpio = test_gpio_addr(data);
unsigned int gpio_id = get_gpio_id(gpio);
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Configure a line as input, raise it, and check that the pin is high */
gpio_set_2bits(gpio, MODER, pin, MODER_INPUT);
@@ -348,7 +351,7 @@ static void test_pull_up_pull_down(const void *data)
uint32_t gpio = test_gpio_addr(data);
unsigned int gpio_id = get_gpio_id(gpio);
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Configure a line as input with pull-up, check the line is set high */
gpio_set_2bits(gpio, MODER, pin, MODER_INPUT);
@@ -378,7 +381,7 @@ static void test_push_pull(const void *data)
uint32_t gpio = test_gpio_addr(data);
uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Setting a line high externally, configuring it in push-pull output */
/* And checking the pin was disconnected */
@@ -425,7 +428,7 @@ static void test_open_drain(const void *data)
uint32_t gpio = test_gpio_addr(data);
uint32_t gpio2 = GPIO_BASE_ADDR + (GPIO_H - gpio);
-qtest_irq_intercept_in(global_qtest, "/machine/soc/syscfg");
+qtest_irq_intercept_in(global_qtest, SYSCFG);
/* Setting a line high externally, configuring it in open-drain output */
/* And checking the pin was disconnected */
diff --git a/tests/qtest/stm32l4x5_syscfg-test.c
b/tests/qtest/stm32l4x5_syscfg-test.c
index 59bac829b7d..506ca08bc24 100644
--- a/tests/qtest/stm32l4x5_syscfg-test.c
+++ b/tests/qtest/stm32l4x5_syscfg-test.c
@@ -1,8 +1,8 @@
/*
* QTest testcase for STM32L4x5_SYSCFG
*
- * Copyright (c) 2023 Arnaud Minier
- * Copyright (c) 2023 Inès Varhol
+ * Copyright (c) 2024 Arnaud Minier
+ * Copyright (c) 2024 Inès Varhol
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -25,6 +25,10 @@
#define SYSCFG_SWPR2 0x28
#define INVALID_ADDR 0x2C
+/* SoC forwards GPIOs to SysCfg */
+#define SYSCFG "/machine/soc"
+#define EXTI "/machine/soc/exti"
+
static void syscfg_writel(unsigned int offset, uint32_t value)
{
writel(SYSCFG_BASE_ADDR + offset, value);
@@ -37,8 +41,7 @@ static uint32_t syscfg_readl(unsigned int offset)
static void syscfg_set_irq(int num, int level)
{
- qtest_set_irq_in(global_qtest, "/machine/soc/syscfg",
-NU
[PULL 12/21] hw/watchdog/sbsa_gwdt: Make watchdog timer frequency a QOM property
Currently the sbsa_gdwt watchdog device hardcodes its frequency at
62.5MHz. In real hardware, this watchdog is supposed to be driven
from the system counter, which also drives the CPU generic timers.
Newer CPU types (in particular from Armv8.6) should have a CPU
generic timer frequency of 1GHz, so we can't leave the watchdog
on the old QEMU default of 62.5GHz.
Make the frequency a QOM property so it can be set by the board,
and have our only board that uses this device set that frequency
to the same value it sets the CPU frequency.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240426122913.3427983-4-peter.mayd...@linaro.org
---
include/hw/watchdog/sbsa_gwdt.h | 3 +--
hw/arm/sbsa-ref.c | 1 +
hw/watchdog/sbsa_gwdt.c | 15 ++-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/include/hw/watchdog/sbsa_gwdt.h b/include/hw/watchdog/sbsa_gwdt.h
index 70b137de301..4bdc6c6fdb6 100644
--- a/include/hw/watchdog/sbsa_gwdt.h
+++ b/include/hw/watchdog/sbsa_gwdt.h
@@ -55,8 +55,6 @@
#define SBSA_GWDT_RMMIO_SIZE 0x1000
#define SBSA_GWDT_CMMIO_SIZE 0x1000
-#define SBSA_TIMER_FREQ 6250 /* Hz */
-
typedef struct SBSA_GWDTState {
/* */
SysBusDevice parent_obj;
@@ -67,6 +65,7 @@ typedef struct SBSA_GWDTState {
qemu_irq irq;
QEMUTimer *timer;
+uint64_t freq;
uint32_t id;
uint32_t wcs;
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 36f6f717b4b..57c337fd92a 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -543,6 +543,7 @@ static void create_wdt(const SBSAMachineState *sms)
SysBusDevice *s = SYS_BUS_DEVICE(dev);
int irq = sbsa_ref_irqmap[SBSA_GWDT_WS0];
+qdev_prop_set_uint64(dev, "clock-frequency", SBSA_GTIMER_HZ);
sysbus_realize_and_unref(s, _fatal);
sysbus_mmio_map(s, 0, rbase);
sysbus_mmio_map(s, 1, cbase);
diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 96895d76369..d437535cc66 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -18,6 +18,7 @@
#include "qemu/osdep.h"
#include "sysemu/reset.h"
#include "sysemu/watchdog.h"
+#include "hw/qdev-properties.h"
#include "hw/watchdog/sbsa_gwdt.h"
#include "qemu/timer.h"
#include "migration/vmstate.h"
@@ -109,7 +110,7 @@ static void sbsa_gwdt_update_timer(SBSA_GWDTState *s,
WdtRefreshType rtype)
timeout = s->woru;
timeout <<= 32;
timeout |= s->worl;
-timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, SBSA_TIMER_FREQ);
+timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, s->freq);
timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
if ((rtype == EXPLICIT_REFRESH) || ((rtype == TIMEOUT_REFRESH) &&
@@ -261,6 +262,17 @@ static void wdt_sbsa_gwdt_realize(DeviceState *dev, Error
**errp)
dev);
}
+static Property wdt_sbsa_gwdt_props[] = {
+/*
+ * Timer frequency in Hz. This must match the frequency used by
+ * the CPU's generic timer. Default 62.5Hz matches QEMU's legacy
+ * CPU timer frequency default.
+ */
+DEFINE_PROP_UINT64("clock-frequency", struct SBSA_GWDTState, freq,
+ 6250),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void wdt_sbsa_gwdt_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -271,6 +283,7 @@ static void wdt_sbsa_gwdt_class_init(ObjectClass *klass,
void *data)
set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
dc->vmsd = _sbsa_gwdt;
dc->desc = "SBSA-compliant generic watchdog device";
+device_class_set_props(dc, wdt_sbsa_gwdt_props);
}
static const TypeInfo wdt_sbsa_gwdt_info = {
--
2.34.1
[PULL 08/21] target/arm: Enable FEAT_Spec_FPACC for -cpu max
FEAT_Spec_FPACC is a feature describing speculative behaviour in the event of a PAC authontication failure when FEAT_FPACCOMBINE is implemented. FEAT_Spec_FPACC means that the speculative use of pointers processed by a PAC Authentication is not materially different in terms of the impact on cached microarchitectural state (caches, TLBs, etc) between passing and failing of the PAC Authentication. QEMU doesn't do speculative execution, so we can advertise this feature. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240418152004.2106516-6-peter.mayd...@linaro.org --- docs/system/arm/emulation.rst | 1 + target/arm/tcg/cpu64.c| 4 2 files changed, 5 insertions(+) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index 307539cff91..7fcea54d8db 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -61,6 +61,7 @@ the following architecture extensions: - FEAT_FP16 (Half-precision floating-point data processing) - FEAT_FPAC (Faulting on AUT* instructions) - FEAT_FPACCOMBINE (Faulting on combined pointer authentication instructions) +- FEAT_FPACC_SPEC (Speculative behavior of combined pointer authentication instructions) - FEAT_FRINTTS (Floating-point to integer instructions) - FEAT_FlagM (Flag manipulation instructions v2) - FEAT_FlagM2 (Enhancements to flag manipulation instructions) diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c index ebb585afd85..443cffe3a85 100644 --- a/target/arm/tcg/cpu64.c +++ b/target/arm/tcg/cpu64.c @@ -1217,6 +1217,10 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64MMFR2, E0PD, 1); /* FEAT_E0PD */ cpu->isar.id_aa64mmfr2 = t; +t = cpu->isar.id_aa64mmfr3; +t = FIELD_DP64(t, ID_AA64MMFR3, SPEC_FPACC, 1); /* FEAT_FPACC_SPEC */ +cpu->isar.id_aa64mmfr3 = t; + t = cpu->isar.id_aa64zfr0; t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 1); t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2); /* FEAT_SVE_PMULL128 */ -- 2.34.1
[PULL 19/21] hw/arm : Create Bl475eMachineState
From: Inès Varhol
Signed-off-by: Arnaud Minier
Signed-off-by: Inès Varhol
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20240424200929.240921-4-ines.var...@telecom-paris.fr
Signed-off-by: Peter Maydell
---
hw/arm/b-l475e-iot01a.c | 46 -
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/hw/arm/b-l475e-iot01a.c b/hw/arm/b-l475e-iot01a.c
index d862aa43fc3..970c637ce61 100644
--- a/hw/arm/b-l475e-iot01a.c
+++ b/hw/arm/b-l475e-iot01a.c
@@ -2,8 +2,8 @@
* B-L475E-IOT01A Discovery Kit machine
* (B-L475E-IOT01A IoT Node)
*
- * Copyright (c) 2023 Arnaud Minier
- * Copyright (c) 2023 Inès Varhol
+ * Copyright (c) 2023-2024 Arnaud Minier
+ * Copyright (c) 2023-2024 Inès Varhol
*
* SPDX-License-Identifier: GPL-2.0-or-later
*
@@ -32,33 +32,51 @@
/* B-L475E-IOT01A implementation is derived from netduinoplus2 */
-static void b_l475e_iot01a_init(MachineState *machine)
+#define TYPE_B_L475E_IOT01A MACHINE_TYPE_NAME("b-l475e-iot01a")
+OBJECT_DECLARE_SIMPLE_TYPE(Bl475eMachineState, B_L475E_IOT01A)
+
+typedef struct Bl475eMachineState {
+MachineState parent_obj;
+
+Stm32l4x5SocState soc;
+} Bl475eMachineState;
+
+static void bl475e_init(MachineState *machine)
{
+Bl475eMachineState *s = B_L475E_IOT01A(machine);
const Stm32l4x5SocClass *sc;
-DeviceState *dev;
-dev = qdev_new(TYPE_STM32L4X5XG_SOC);
-object_property_add_child(OBJECT(machine), "soc", OBJECT(dev));
-sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+object_initialize_child(OBJECT(machine), "soc", >soc,
+TYPE_STM32L4X5XG_SOC);
+sysbus_realize(SYS_BUS_DEVICE(>soc), _fatal);
-sc = STM32L4X5_SOC_GET_CLASS(dev);
-armv7m_load_kernel(ARM_CPU(first_cpu),
- machine->kernel_filename,
- 0, sc->flash_size);
+sc = STM32L4X5_SOC_GET_CLASS(>soc);
+armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, 0,
+ sc->flash_size);
}
-static void b_l475e_iot01a_machine_init(MachineClass *mc)
+static void bl475e_machine_init(ObjectClass *oc, void *data)
{
+MachineClass *mc = MACHINE_CLASS(oc);
static const char *machine_valid_cpu_types[] = {
ARM_CPU_TYPE_NAME("cortex-m4"),
NULL
};
mc->desc = "B-L475E-IOT01A Discovery Kit (Cortex-M4)";
-mc->init = b_l475e_iot01a_init;
+mc->init = bl475e_init;
mc->valid_cpu_types = machine_valid_cpu_types;
/* SRAM pre-allocated as part of the SoC instantiation */
mc->default_ram_size = 0;
}
-DEFINE_MACHINE("b-l475e-iot01a", b_l475e_iot01a_machine_init)
+static const TypeInfo bl475e_machine_type[] = {
+{
+.name = TYPE_B_L475E_IOT01A,
+.parent = TYPE_MACHINE,
+.instance_size = sizeof(Bl475eMachineState),
+.class_init = bl475e_machine_init,
+}
+};
+
+DEFINE_TYPES(bl475e_machine_type)
--
2.34.1
[PULL 06/21] target/arm: Enable FEAT_ETS2 for -cpu max
FEAT_ETS2 is a tighter set of guarantees about memory ordering involving translation table walks than the old FEAT_ETS; FEAT_ETS has been retired from the Arm ARM and the old ID_AA64MMFR1.ETS == 1 now gives no greater guarantees than ETS == 0. FEAT_ETS2 requires: * the virtual address of a load or store that appears in program order after a DSB cannot be translated until after the DSB completes (section B2.10.9) * TLB maintenance operations that only affect translations without execute permission are guaranteed complete after a DSB (R_BLDZX) * if a memory access RW2 is ordered-before memory access RW2, then RW1 is also ordered-before any translation table walk generated by RW2 that generates a Translation, Address size or Access flag fault (R_NNFPF, I_CLGHP) As with FEAT_ETS, QEMU is already compliant, because we do not reorder translation table walk memory accesses relative to other memory accesses, and we always guarantee to have finished TLB maintenance as soon as the TLB op is done. Update the documentation to list FEAT_ETS2 instead of the no-longer-existent FEAT_ETS, and update the 'max' CPU ID registers. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240418152004.2106516-4-peter.mayd...@linaro.org --- docs/system/arm/emulation.rst | 2 +- target/arm/tcg/cpu32.c| 2 +- target/arm/tcg/cpu64.c| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index d70b66f7530..307539cff91 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -50,7 +50,7 @@ the following architecture extensions: - FEAT_EL2 (Support for execution at EL2) - FEAT_EL3 (Support for execution at EL3) - FEAT_EPAC (Enhanced pointer authentication) -- FEAT_ETS (Enhanced Translation Synchronization) +- FEAT_ETS2 (Enhanced Translation Synchronization) - FEAT_EVT (Enhanced Virtualization Traps) - FEAT_F32MM (Single-precision Matrix Multiplication) - FEAT_F64MM (Double-precision Matrix Multiplication) diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c index de8f2be9416..b5a60682fa6 100644 --- a/target/arm/tcg/cpu32.c +++ b/target/arm/tcg/cpu32.c @@ -67,7 +67,7 @@ void aa32_max_features(ARMCPU *cpu) cpu->isar.id_mmfr4 = t; t = cpu->isar.id_mmfr5; -t = FIELD_DP32(t, ID_MMFR5, ETS, 1); /* FEAT_ETS */ +t = FIELD_DP32(t, ID_MMFR5, ETS, 2); /* FEAT_ETS2 */ cpu->isar.id_mmfr5 = t; t = cpu->isar.id_pfr0; diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c index 8ad05c53e8d..ebb585afd85 100644 --- a/target/arm/tcg/cpu64.c +++ b/target/arm/tcg/cpu64.c @@ -1196,7 +1196,7 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1); /* FEAT_LOR */ t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 3); /* FEAT_PAN3 */ t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1); /* FEAT_XNX */ -t = FIELD_DP64(t, ID_AA64MMFR1, ETS, 1); /* FEAT_ETS */ +t = FIELD_DP64(t, ID_AA64MMFR1, ETS, 2); /* FEAT_ETS2 */ t = FIELD_DP64(t, ID_AA64MMFR1, HCX, 1); /* FEAT_HCX */ t = FIELD_DP64(t, ID_AA64MMFR1, TIDCP1, 1); /* FEAT_TIDCP1 */ cpu->isar.id_aa64mmfr1 = t; -- 2.34.1
[PULL 09/21] tests/avocado: update sunxi kernel from armbian to 6.6.16
The Linux kernel 5.10.16 binary for sunxi has been removed from
apt.armbian.com. This means that the avocado tests for these machines
will be skipped (status CANCEL) if the old binary isn't present in
the avocado cache.
Update to 6.6.16, in the same way we did in commit e384db41d8661
when we moved to 5.10.16 in 2021.
Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2284
Signed-off-by: Peter Maydell
Reviewed-by: Strahinja Jankovic
Reviewed-by: Niek Linnenbank
Tested-by: Niek Linnenbank
Message-id: 20240415151845.1564201-1-peter.mayd...@linaro.org
---
tests/avocado/boot_linux_console.py | 70 ++---
tests/avocado/replay_kernel.py | 8 ++--
2 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/tests/avocado/boot_linux_console.py
b/tests/avocado/boot_linux_console.py
index 180ac17326e..c35fc5e9ba2 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -646,12 +646,12 @@ def test_arm_cubieboard_initrd(self):
:avocado: tags=accel:tcg
"""
deb_url = ('https://apt.armbian.com/pool/main/l/'
-
'linux-5.10.16-sunxi/linux-image-current-sunxi_21.02.2_armhf.deb')
-deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
+
'linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb')
+deb_hash = 'f7c3c8c5432f765445dc6e7eab02f3bbe668256b'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
-'/boot/vmlinuz-5.10.16-sunxi')
-dtb_path =
'/usr/lib/linux-image-current-sunxi/sun4i-a10-cubieboard.dtb'
+
'/boot/vmlinuz-6.6.16-current-sunxi')
+dtb_path =
'/usr/lib/linux-image-6.6.16-current-sunxi/sun4i-a10-cubieboard.dtb'
dtb_path = self.extract_from_deb(deb_path, dtb_path)
initrd_url = ('https://github.com/groeck/linux-build-test/raw/'
'2eb0a73b5d5a28df3170c546ddaaa9757e1e0848/rootfs/'
@@ -690,12 +690,12 @@ def test_arm_cubieboard_sata(self):
:avocado: tags=accel:tcg
"""
deb_url = ('https://apt.armbian.com/pool/main/l/'
-
'linux-5.10.16-sunxi/linux-image-current-sunxi_21.02.2_armhf.deb')
-deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
+
'linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb')
+deb_hash = 'f7c3c8c5432f765445dc6e7eab02f3bbe668256b'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
-'/boot/vmlinuz-5.10.16-sunxi')
-dtb_path =
'/usr/lib/linux-image-current-sunxi/sun4i-a10-cubieboard.dtb'
+
'/boot/vmlinuz-6.6.16-current-sunxi')
+dtb_path =
'/usr/lib/linux-image-6.6.16-current-sunxi/sun4i-a10-cubieboard.dtb'
dtb_path = self.extract_from_deb(deb_path, dtb_path)
rootfs_url = ('https://github.com/groeck/linux-build-test/raw/'
'2eb0a73b5d5a28df3170c546ddaaa9757e1e0848/rootfs/'
@@ -872,13 +872,13 @@ def test_arm_bpim2u(self):
:avocado: tags=machine:bpim2u
:avocado: tags=accel:tcg
"""
-deb_url = ('https://apt.armbian.com/pool/main/l/linux-5.10.16-sunxi/'
- 'linux-image-current-sunxi_21.02.2_armhf.deb')
-deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
+deb_url = ('https://apt.armbian.com/pool/main/l/'
+
'linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb')
+deb_hash = 'f7c3c8c5432f765445dc6e7eab02f3bbe668256b'
deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
kernel_path = self.extract_from_deb(deb_path,
-'/boot/vmlinuz-5.10.16-sunxi')
-dtb_path = ('/usr/lib/linux-image-current-sunxi/'
+
'/boot/vmlinuz-6.6.16-current-sunxi')
+dtb_path = ('/usr/lib/linux-image-6.6.16-current-sunxi/'
'sun8i-r40-bananapi-m2-ultra.dtb')
dtb_path = self.extract_from_deb(deb_path, dtb_path)
@@ -899,13 +899,13 @@ def test_arm_bpim2u_initrd(self):
:avocado: tags=accel:tcg
:avocado: tags=machine:bpim2u
"""
-deb_url = ('https://apt.armbian.com/pool/main/l/linux-5.10.16-sunxi/'
- 'linux-image-current-sunxi_21.02.2_armhf.deb')
-deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
+deb_url = ('https://apt.armbian.com/pool/main/l/'
+
'linux-6.6.16/linu
[PULL 16/21] hw/arm/npcm7xx: Store derivative OTP fuse key in little endian
From: Philippe Mathieu-Daudé
Use little endian for derivative OTP fuse key.
Cc: qemu-sta...@nongnu.org
Fixes: c752bb079b ("hw/nvram: NPCM7xx OTP device model")
Suggested-by: Avi Fishman
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20240422125813.1403-1-phi...@linaro.org
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/arm/npcm7xx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index cc68b5d8f12..9f2d96c733a 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -24,6 +24,7 @@
#include "hw/qdev-clock.h"
#include "hw/qdev-properties.h"
#include "qapi/error.h"
+#include "qemu/bswap.h"
#include "qemu/units.h"
#include "sysemu/sysemu.h"
#include "target/arm/cpu-qom.h"
@@ -386,7 +387,7 @@ static void npcm7xx_init_fuses(NPCM7xxState *s)
* The initial mask of disabled modules indicates the chip derivative (e.g.
* NPCM750 or NPCM730).
*/
-value = tswap32(nc->disabled_modules);
+value = cpu_to_le32(nc->disabled_modules);
npcm7xx_otp_array_write(>fuse_array, , NPCM7XX_FUSE_DERIVATIVE,
sizeof(value));
}
--
2.34.1
[PULL 05/21] target/arm: Enable FEAT_CSV2_3 for -cpu max
FEAT_CSV2_3 adds a mechanism to identify if hardware cannot disclose information about whether branch targets and branch history trained in one hardware described context can control speculative execution in a different hardware context. There is no branch prediction in TCG, so we don't need to do anything to be compliant with this. Upadte the '-cpu max' ID registers to advertise the feature. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-id: 20240418152004.2106516-3-peter.mayd...@linaro.org --- docs/system/arm/emulation.rst | 1 + target/arm/tcg/cpu64.c| 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index 5fdc64a944f..d70b66f7530 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -32,6 +32,7 @@ the following architecture extensions: - FEAT_CSV2_1p1 (Cache speculation variant 2, version 1.1) - FEAT_CSV2_1p2 (Cache speculation variant 2, version 1.2) - FEAT_CSV2_2 (Cache speculation variant 2, version 2) +- FEAT_CSV2_3 (Cache speculation variant 2, version 3) - FEAT_CSV3 (Cache speculation variant 3) - FEAT_DGH (Data gathering hint) - FEAT_DIT (Data Independent Timing instructions) diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c index 62c4663512b..8ad05c53e8d 100644 --- a/target/arm/tcg/cpu64.c +++ b/target/arm/tcg/cpu64.c @@ -1159,7 +1159,7 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1); t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1); /* FEAT_SEL2 */ t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1); /* FEAT_DIT */ -t = FIELD_DP64(t, ID_AA64PFR0, CSV2, 2); /* FEAT_CSV2_2 */ +t = FIELD_DP64(t, ID_AA64PFR0, CSV2, 3); /* FEAT_CSV2_3 */ t = FIELD_DP64(t, ID_AA64PFR0, CSV3, 1); /* FEAT_CSV3 */ cpu->isar.id_aa64pfr0 = t; @@ -1174,7 +1174,7 @@ void aarch64_max_tcg_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64PFR1, MTE, 3); /* FEAT_MTE3 */ t = FIELD_DP64(t, ID_AA64PFR1, RAS_FRAC, 0); /* FEAT_RASv1p1 + FEAT_DoubleFault */ t = FIELD_DP64(t, ID_AA64PFR1, SME, 1); /* FEAT_SME */ -t = FIELD_DP64(t, ID_AA64PFR1, CSV2_FRAC, 0); /* FEAT_CSV2_2 */ +t = FIELD_DP64(t, ID_AA64PFR1, CSV2_FRAC, 0); /* FEAT_CSV2_3 */ t = FIELD_DP64(t, ID_AA64PFR1, NMI, 1); /* FEAT_NMI */ cpu->isar.id_aa64pfr1 = t; -- 2.34.1
[PULL 02/21] hvf: arm: Remove PL1_WRITE_MASK
From: Zenghui Yu
As it had never been used since the first commit a1477da3ddeb ("hvf: Add
Apple Silicon support").
Signed-off-by: Zenghui Yu
Message-id: 20240422092715.71973-1-zenghui...@linux.dev
Reviewed-by: Peter Maydell
Signed-off-by: Peter Maydell
---
target/arm/hvf/hvf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index db628c1cba7..8e942f89b35 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -150,7 +150,6 @@ void hvf_arm_init_debug(void)
#define HVF_SYSREG(crn, crm, op0, op1, op2) \
ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP, crn, crm, op0, op1, op2)
-#define PL1_WRITE_MASK 0x4
#define SYSREG_OP0_SHIFT 20
#define SYSREG_OP0_MASK 0x3
--
2.34.1
Re: [PATCH v6 0/5] Add device DM163 (led driver, matrix colors shield & display)
On Wed, 24 Apr 2024 at 21:09, Inès Varhol wrote: > > This device implements the IM120417002 colors shield v1.1 for Arduino > (which relies on the DM163 8x3-channel led driving logic) and features > a simple display of an 8x8 RGB matrix. This color shield can be plugged > on the Arduino board (or the B-L475E-IOT01A board) to drive an 8x8 > RGB led matrix. This RGB led matrix takes advantage of retinal persistance > to seemingly display different colors in each row. > Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v4] fix endianness bug
On Sun, 28 Apr 2024 at 19:12, Alexandra Diupina wrote:
>
> Add xlnx_dpdma_read_descriptor() and
> xlnx_dpdma_write_descriptor() functions.
> xlnx_dpdma_read_descriptor() combines reading a
> descriptor from desc_addr by calling dma_memory_read()
> and swapping the desc fields from guest memory order
> to host memory order. xlnx_dpdma_write_descriptor()
> performs similar actions when writing a descriptor.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d3c6369a96 ("introduce xlnx-dpdma")
> Signed-off-by: Alexandra Diupina
> ---
> v4: remove rewriting desc in place
> v3: add xlnx_dpdma_write_descriptor()
> v2: minor changes in xlnx_dpdma_read_descriptor()
> hw/dma/xlnx_dpdma.c | 63 ++---
> 1 file changed, 59 insertions(+), 4 deletions(-)
>
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index 1f5cd64ed1..e9133e9dcb 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -614,6 +614,63 @@ static void xlnx_dpdma_register_types(void)
> type_register_static(_dpdma_info);
> }
>
> +static MemTxResult xlnx_dpdma_read_descriptor(XlnxDPDMAState *s,
> +uint64_t desc_addr, DPDMADescriptor
> *desc)
> +{
> +if (dma_memory_read(_space_memory, desc_addr, ,
> +sizeof(DPDMADescriptor),
> MEMTXATTRS_UNSPECIFIED)) {
> +return MEMTX_ERROR;
You should return the return value you got from dma_memory_read() here.
> +}
> +
> +/* Convert from LE into host endianness. */
> +desc->control = le32_to_cpu(desc->control);
> +desc->descriptor_id = le32_to_cpu(desc->descriptor_id);
> +desc->xfer_size = le32_to_cpu(desc->xfer_size);
> +desc->line_size_stride = le32_to_cpu(desc->line_size_stride);
> +desc->timestamp_lsb = le32_to_cpu(desc->timestamp_lsb);
> +desc->timestamp_msb = le32_to_cpu(desc->timestamp_msb);
> +desc->address_extension = le32_to_cpu(desc->address_extension);
> +desc->next_descriptor = le32_to_cpu(desc->next_descriptor);
> +desc->source_address = le32_to_cpu(desc->source_address);
> +desc->address_extension_23 = le32_to_cpu(desc->address_extension_23);
> +desc->address_extension_45 = le32_to_cpu(desc->address_extension_45);
> +desc->source_address2 = le32_to_cpu(desc->source_address2);
> +desc->source_address3 = le32_to_cpu(desc->source_address3);
> +desc->source_address4 = le32_to_cpu(desc->source_address4);
> +desc->source_address5 = le32_to_cpu(desc->source_address5);
> +desc->crc = le32_to_cpu(desc->crc);
> +
> +return MEMTX_OK;
> +}
> +
> +static void xlnx_dpdma_write_descriptor(uint64_t desc_addr,
> +DPDMADescriptor *desc)
> +{
> +DPDMADescriptor* tmp_desc = (DPDMADescriptor
> *)malloc(sizeof(DPDMADescriptor));
> +memcpy(tmp_desc, desc, sizeof(desc));
The descriptor structure is not very big, we don't need to malloc it.
So we can do:
DPDMADescriptor tmp_desc = *desc;
(adjusting the code below to match).
> +
> +/* Convert from host endianness into LE. */
> +tmp_desc->control = cpu_to_le32(tmp_desc->control);
> +tmp_desc->descriptor_id = cpu_to_le32(tmp_desc->descriptor_id);
> +tmp_desc->xfer_size = cpu_to_le32(tmp_desc->xfer_size);
> +tmp_desc->line_size_stride = cpu_to_le32(tmp_desc->line_size_stride);
> +tmp_desc->timestamp_lsb = cpu_to_le32(tmp_desc->timestamp_lsb);
> +tmp_desc->timestamp_msb = cpu_to_le32(tmp_desc->timestamp_msb);
> +tmp_desc->address_extension = cpu_to_le32(tmp_desc->address_extension);
> +tmp_desc->next_descriptor = cpu_to_le32(tmp_desc->next_descriptor);
> +tmp_desc->source_address = cpu_to_le32(tmp_desc->source_address);
> +tmp_desc->address_extension_23 =
> cpu_to_le32(tmp_desc->address_extension_23);
> +tmp_desc->address_extension_45 =
> cpu_to_le32(tmp_desc->address_extension_45);
> +tmp_desc->source_address2 = cpu_to_le32(tmp_desc->source_address2);
> +tmp_desc->source_address3 = cpu_to_le32(tmp_desc->source_address3);
> +tmp_desc->source_address4 = cpu_to_le32(tmp_desc->source_address4);
> +tmp_desc->source_address5 = cpu_to_le32(tmp_desc->source_address5);
> +tmp_desc->crc = cpu_to_le32(tmp_desc->crc);
> +
> +dma_memory_write(_space_memory, desc_addr, tmp_desc,
> +sizeof(DPDMADescriptor), MEMTXATTRS_UNSPECIFIED);
I know we don't check the return value at the callsite, but we might
as well do "return dma_memory_write(...)" here.
> +}
> +
> size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s, uint8_t channel,
> bool one_desc)
> {
> @@ -651,8 +708,7 @@ size_t xlnx_dpdma_start_operation(XlnxDPDMAState *s,
> uint8_t channel,
> desc_addr = xlnx_dpdma_descriptor_next_address(s, channel);
> }
>
> -if (dma_memory_read(_space_memory, desc_addr, ,
> -
Re: [PATCH] hw/arm/npcm7xx: Store derivative OTP fuse key in little endian
On Mon, 22 Apr 2024 at 13:58, Philippe Mathieu-Daudé wrote:
>
> Use little endian for derivative OTP fuse key.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: c752bb079b ("hw/nvram: NPCM7xx OTP device model")
> Suggested-by: Avi Fishman
> Signed-off-by: Philippe Mathieu-Daudé
Applied to target-arm.next, thanks.
-- PMM
Re: [PATCH] hw/char/stm32l4x5_usart: Fix memory corruption by adding correct class_size
On Mon, 29 Apr 2024 at 08:59, Thomas Huth wrote:
>
> "make check-qtest-aarch64" recently started failing on FreeBSD builds,
> and valgrind on Linux also detected that there is something fishy with
> the new stm32l4x5-usart: The code forgot to set the correct class_size
> here, so the various class_init functions in this file wrote beyond
> the allocated buffer when setting the subc->type field.
>
> Fixes: 4fb37aea7e ("hw/char: Implement STM32L4x5 USART skeleton")
> Signed-off-by: Thomas Huth
> ---
Applied to target-arm.next, thanks.
-- PMM
Re: [PATCH v2] fix bit fields extraction and prevent overflow
On Sun, 28 Apr 2024 at 19:11, Alexandra Diupina wrote:
>
> Add a type cast and use extract64() instead of extract32()
> to avoid integer overflow on addition. Fix bit fields
> extraction according to documentation.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d3c6369a96 ("introduce xlnx-dpdma")
> Signed-off-by: Alexandra Diupina
Thanks; I've applied this to target-arm.next, and it'll go into
a pullreq sometime this week. (I tweaked the commit message to
add a bit of the context and the docs URL from the other
email thread.)
-- PMM
Re: [PATCH] tests/avocado: update sunxi kernel from armbian to 6.6.16
On Mon, 29 Apr 2024 at 21:40, Niek Linnenbank wrote: > > Hi Peter, Strahinja, > > I can confirm that the orangepi-pc and cubieboard based tests are working OK > using the newer kernel 6.6.16: > > $ ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes > ./build/pyvenv/bin/avocado --show=app,console run -t machine:orangepi-pc -t > machine:cubieboard tests/avocado/boot_linux_console.py > ... > RESULTS: PASS 7 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | > CANCEL 1 > JOB TIME : 177.65 s > > So for this patch: > Reviewed-by: Niek Linnenbank > Tested-by: Niek Linnenbank Great, thanks. (I'll put this patch into an upcoming arm pullreq.) > About the BootLinuxConsole.test_arm_orangepi_bionic_20_08 test, I'd be happy > to provide a patch to revive that test. > Since that test is no longer working without having the image available, this > could also be a good moment to re-consider if armbian is really the best > input for testing > the orangepi-pc board. The image is relatively larger and slower compared to > other images, like the two openwrt based tests for cubieboard and bpim2u. > > After some searching I've found that Openwrt also has orangepi-pc support: > https://openwrt.org/toh/xunlong/orange_pi_pc > > That image works fine with our emulated orangepi-pc board: > > $ qemu-system-arm -M orangepi-pc -sd > openwrt-23.05.0-sunxi-cortexa7-xunlong_orangepi-pc-ext4-sdcard.img -nographic > Using openwrt also for the orangepi-pc test instead of armbian also gives > some consistency between the various tests, to some degree. What are you > opinions on this? Yeah, seems reasonable. My main thing to think about would be that to understand what extra coverage this gives us that we don't already have (there's no point running a ton of tests which all amount to "boot a Linux kernel to a shell prompt"). It looks like what we get from this one is that we are testing the "boot off an SD card image via u-boot" flow -- is that right? thanks -- PMM
[PATCH 2/2] target/arm: Implement FEAT WFxT and enable for '-cpu max'
FEAT_WFxT introduces new instructions WFIT and WFET, which are like
the existing WFI and WFE but allow the guest to pass a timeout value
in a register. The instructions will wait for an interrupt/event as
usual, but will also stop waiting when the value of CNTVCT_EL0 is
greater than or equal to the specified timeout value.
We implement WFIT by setting up a timer to expire at the right
point; when the timer expires it sets the EXITTB interrupt, which
will cause the CPU to leave the halted state. If we come out of
halt for some other reason, we unset the pending timer.
We implement WFET as a nop, which is architecturally permitted and
matches the way we currently make WFE a nop.
Signed-off-by: Peter Maydell
---
docs/system/arm/emulation.rst | 1 +
target/arm/cpu-features.h | 5
target/arm/cpu.h | 3 ++
target/arm/helper.h| 1 +
target/arm/internals.h | 8 +
target/arm/tcg/a64.decode | 4 +++
target/arm/cpu.c | 38
target/arm/helper.c| 4 +--
target/arm/machine.c | 20 +
target/arm/tcg/cpu64.c | 1 +
target/arm/tcg/op_helper.c | 54 ++
target/arm/tcg/translate-a64.c | 41 ++
12 files changed, 178 insertions(+), 2 deletions(-)
diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index a9ae7ede9fc..d283c985d14 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -108,6 +108,7 @@ the following architecture extensions:
- FEAT_UAO (Unprivileged Access Override control)
- FEAT_VHE (Virtualization Host Extensions)
- FEAT_VMID16 (16-bit VMID)
+- FEAT_WFxT (WFE and WFI instructions with timeout)
- FEAT_XNX (Translation table stage 2 Unprivileged Execute-never)
- SVE (The Scalable Vector Extension)
- SVE2 (The Scalable Vector Extension v2)
diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index b300d0446d8..c59ca104fe1 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -571,6 +571,11 @@ static inline bool isar_feature_aa64_i8mm(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, I8MM) != 0;
}
+static inline bool isar_feature_aa64_wfxt(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, WFXT) >= 2;
+}
+
static inline bool isar_feature_aa64_hbc(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64isar2, ID_AA64ISAR2, BC) != 0;
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 97997dbd087..e8e6024fe30 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -868,6 +868,9 @@ struct ArchCPU {
* pmu_op_finish() - it does not need other handling during migration
*/
QEMUTimer *pmu_timer;
+/* Timer used for WFxT timeouts */
+QEMUTimer *wfxt_timer;
+
/* GPIO outputs for generic timer */
qemu_irq gt_timer_outputs[NUM_GTIMERS];
/* GPIO output for GICv3 maintenance interrupt signal */
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 2b027333053..a85de78c8fc 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -53,6 +53,7 @@ DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl)
DEF_HELPER_1(setend, void, env)
DEF_HELPER_2(wfi, void, env, i32)
DEF_HELPER_1(wfe, void, env)
+DEF_HELPER_2(wfit, void, env, i64)
DEF_HELPER_1(yield, void, env)
DEF_HELPER_1(pre_hvc, void, env)
DEF_HELPER_2(pre_smc, void, env, i32)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index b53f5e8ff2a..bd32883890d 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1760,4 +1760,12 @@ bool check_watchpoint_in_range(int i, target_ulong addr);
CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr);
int insert_hw_watchpoint(target_ulong addr, target_ulong len, int type);
int delete_hw_watchpoint(target_ulong addr, target_ulong len, int type);
+
+/* Return the current value of the system counter in ticks */
+uint64_t gt_get_countervalue(CPUARMState *env);
+/*
+ * Return the currently applicable offset between the system counter
+ * and CNTVCT_EL0 (this will be either 0 or the value of CNTVOFF_EL2).
+ */
+uint64_t gt_virt_cnt_offset(CPUARMState *env);
#endif
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 0e7656fd158..7aea5cba5ea 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -183,6 +183,10 @@ ERETA 1101011 0100 1 1 m:1 1 1
# ERETAA, ERETAB
NOP 1101 0101 0011 0010 --- 1
}
+# System instructions with register argument
+WFET1101 0101 0011 0001 000 rd:5
+WFIT1101 0101 0011 0001 001 rd:5
+
# Barriers
CLREX 1101 0101 0011 0011 010 1
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a152def2413..006092a6b12 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1132,6 +1132,33 @@
[PATCH 1/2] accel/tcg: Make TCGCPUOps::cpu_exec_halt return bool for whether to halt
The TCGCPUOps::cpu_exec_halt method is called from cpu_handle_halt()
when the CPU is halted, so that a target CPU emulation can do
anything target-specific it needs to do. (At the moment we only use
this on i386.)
The current specification of the method doesn't allow the target
specific code to do something different if the CPU is about to come
out of the halt state, because cpu_handle_halt() only determines this
after the method has returned. (If the method called cpu_has_work()
itself this would introduce a potential race if an interrupt arrived
between the target's method implementation checking and
cpu_handle_halt() repeating the check.)
Change the definition of the method so that it returns a bool to
tell cpu_handle_halt() whether to stay in halt or not.
We will want this for the Arm target, where FEAT_WFxT wants to do
some work only for the case where the CPU is in halt but about to
leave it.
Signed-off-by: Peter Maydell
---
include/hw/core/tcg-cpu-ops.h | 11 +--
target/i386/tcg/helper-tcg.h| 2 +-
accel/tcg/cpu-exec.c| 7 +--
target/i386/tcg/sysemu/seg_helper.c | 3 ++-
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index dc1f16a9777..f3ac76e6f6d 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -111,8 +111,15 @@ struct TCGCPUOps {
void (*do_interrupt)(CPUState *cpu);
/** @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec */
bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
-/** @cpu_exec_halt: Callback for handling halt in cpu_exec */
-void (*cpu_exec_halt)(CPUState *cpu);
+/**
+ * @cpu_exec_halt: Callback for handling halt in cpu_exec.
+ *
+ * Return true to indicate that the CPU should now leave halt, false
+ * if it should remain in the halted state.
+ * If this method is not provided, the default is to leave halt
+ * if cpu_has_work() returns true.
+ */
+bool (*cpu_exec_halt)(CPUState *cpu);
/**
* @tlb_fill: Handle a softmmu tlb miss
*
diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index effc2c1c984..85957943bf3 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -39,7 +39,7 @@ QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS >
TARGET_PHYS_ADDR_SPACE_BITS);
*/
void x86_cpu_do_interrupt(CPUState *cpu);
#ifndef CONFIG_USER_ONLY
-void x86_cpu_exec_halt(CPUState *cpu);
+bool x86_cpu_exec_halt(CPUState *cpu);
bool x86_need_replay_interrupt(int interrupt_request);
bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
#endif
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5c70748060a..550f93b19ce 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -669,11 +669,14 @@ static inline bool cpu_handle_halt(CPUState *cpu)
#ifndef CONFIG_USER_ONLY
if (cpu->halted) {
const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops;
+bool leave_halt;
if (tcg_ops->cpu_exec_halt) {
-tcg_ops->cpu_exec_halt(cpu);
+leave_halt = tcg_ops->cpu_exec_halt(cpu);
+} else {
+leave_halt = cpu_has_work(cpu);
}
-if (!cpu_has_work(cpu)) {
+if (!leave_halt) {
return true;
}
diff --git a/target/i386/tcg/sysemu/seg_helper.c
b/target/i386/tcg/sysemu/seg_helper.c
index 2db8083748e..9ba94deb3aa 100644
--- a/target/i386/tcg/sysemu/seg_helper.c
+++ b/target/i386/tcg/sysemu/seg_helper.c
@@ -128,7 +128,7 @@ void x86_cpu_do_interrupt(CPUState *cs)
}
}
-void x86_cpu_exec_halt(CPUState *cpu)
+bool x86_cpu_exec_halt(CPUState *cpu)
{
if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
X86CPU *x86_cpu = X86_CPU(cpu);
@@ -138,6 +138,7 @@ void x86_cpu_exec_halt(CPUState *cpu)
cpu_reset_interrupt(cpu, CPU_INTERRUPT_POLL);
bql_unlock();
}
+return cpu_has_work(cpu);
}
bool x86_need_replay_interrupt(int interrupt_request)
--
2.34.1
[PATCH 0/2] target/arm: Implement FEAT_WFxT
FEAT_WFxT introduces new instructions WFIT and WFET, which are like the existing WFI and WFE but allow the guest to pass a timeout value in a register. The instructions will wait for an interrupt/event as usual, but will also stop waiting when the value of CNTVCT_EL0 is greater than or equal to the specified timeout value. This series implements this and enables it for '-cpu max'. Patch 1 is a tweak to the TCGCPUOps::cpu_exec_halt method so that we can use it in patch 2 for "do some work when we are going to leave the halt state". thanks -- PMM Peter Maydell (2): accel/tcg: Make TCGCPUOps::cpu_exec_halt return bool for whether to halt target/arm: Implement FEAT WFxT and enable for '-cpu max' docs/system/arm/emulation.rst | 1 + include/hw/core/tcg-cpu-ops.h | 11 -- target/arm/cpu-features.h | 5 +++ target/arm/cpu.h| 3 ++ target/arm/helper.h | 1 + target/arm/internals.h | 8 + target/i386/tcg/helper-tcg.h| 2 +- target/arm/tcg/a64.decode | 4 +++ accel/tcg/cpu-exec.c| 7 ++-- target/arm/cpu.c| 38 target/arm/helper.c | 4 +-- target/arm/machine.c| 20 +++ target/arm/tcg/cpu64.c | 1 + target/arm/tcg/op_helper.c | 54 + target/arm/tcg/translate-a64.c | 41 ++ target/i386/tcg/sysemu/seg_helper.c | 3 +- 16 files changed, 195 insertions(+), 8 deletions(-) -- 2.34.1
Re: [PATCH v5 00/14] tpm: introduce TPM CRB SysBus device
On Tue, 14 Nov 2023 at 02:10, Joelle van Dyne wrote: > The impetus for this patch set is to get TPM 2.0 working on Windows 11 ARM64. > Windows' tpm.sys does not seem to work on a TPM TIS device (as verified with > VMWare's implementation). However, the current TPM CRB device uses a fixed > system bus address that is reserved for RAM in ARM64 Virt machines. > > In the process of adding the TPM CRB SysBus device, we also went ahead and > cleaned up some of the existing TPM hardware code and fixed some bugs. We used > the TPM TIS devices as a template for the TPM CRB devices and refactored out > common code. We moved the ACPI DSDT generation to the device in order to > handle > dynamic base address requirements as well as reduce redundent code in > different > machine ACPI generation. We also changed the tpm_crb device to use the ISA bus > instead of depending on the default system bus as the device only was built > for > the PC configuration. > > Another change is that the TPM CRB registers are now mapped in the same way > that > the pflash ROM devices are mapped. It is a memory region whose writes are > trapped as MMIO accesses. This was needed because Apple Silicon does not > decode > LDP (AARCH64 load pair of registers) caused page faults. @agraf suggested that > we do this to avoid having to do AARCH64 decoding in the HVF backend's fault > handler. I had a conversation about this on IRC a week or so back (though I forget who with, sorry) that made me realise there's a problem with this approach, and I wanted to write that up for the mailing list. The problem with turning this into a memory-backed device rather than an MMIO backed device is that it breaks KVM on Arm CPUs which don't have FEAT_S2FWB (i.e. anything older than Armv8.4). This is because without FEAT_S2FWB the guest and host will disagree about the memory attributes of the region: * the host knows this is RAM backed and it's normal-cacheable (certainly as far as the mapping that QEMU itself has) * the guest thinks it's real hardware device registers and maps it as Device The resulting mismatch in cacheability attributes can cause unexpected behaviour where the guest and QEMU views of the memory contents don't necessarily match up. (This is the same underlying issue that means that you can't use QEMU devices that emulate VGA framebuffers on AArch64 KVM without FEAT_S2FWB.) With FEAT_S2FWB the problem goes away because the hypervisor can override the guest's specified memory attributes to get rid of the attribute mismatch. So given that this would cause a regression for KVM, my preference is to stick with the current "the device is backed by MMIO read and write functions in the normal way". If a particular guest is trying to access it with LDP/STP that is best fixed in the guest. Potentially we could emulate (interpret) some subset of complex load/store insns in QEMU at the point where we get the "took a data abort but no syndrome information". This ought to be doable in a way that's shareable between hvf and KVM, and we can write a decodetree file for the insns we want to interpret. (I would not try to share with TCG decodetree, though the patterns can probably be copied across.) thanks -- PMM
Re: QEMU Community Call Agenda Items (April 30th, 2024)
On Tue, 30 Apr 2024 at 08:36, Markus Armbruster wrote: > > Daniel P. Berrangé writes: > > * /machine to get the rtc-time property value > > This is an alias to the RTC device's "rtc-time" property. Only some > machines define it. Useful because the actual property depends on > machine type and configuration. For q35, it's > /machine/unattached/device[N]/rtc/date, where N can vary. > > If we moved the southbridge out of the /machine/unattached dump, we'd > have something like /machine/q35/ich9-lpc/rtc/date. Stable, but you > have to know the machine type to find it. Do we really want to call that stable, though? If we ever wanted to refactor the devices internally it might change. My gut feeling is that exposing something we want to be stable as a specific "this is obviously an externally exposed identifier" (e.g. in the way we do by having an rtc-time property alias on the machine object) is more likely to be reliable than trusting that a QOM path all the way down to a specific device is never going to be rearranged. thanks -- PMM
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, 30 Apr 2024 at 07:45, Thomas Huth wrote: > > Old machine types often have bugs or work-arounds that affect our > possibilities to move forward with the QEMU code base (see for example > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > cannot be fixed without breaking live migration with old machine types, > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > commit ea985d235b86). So instead of going through the process of manually > deprecating old machine types again and again, let's rather add an entry > that can stay, which declares that machine types older than 6 years are > considered as deprecated automatically. Six years should be sufficient to > support the release cycles of most Linux distributions. Sounds like a good idea. > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 6d595de3b6..fe69e2d44c 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. > System emulator machines > > > +Versioned machine types older than 6 years "more than 6 years old" > +'' > + > +Starting with the release of QEMU 10.0, versioned machine types older than > +6 years ditto will automatically be considered as deprecated and might be due to > +removal without furthor notice. For example, this affects machine types like "further" > +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y where > +X is the major number and Y is the minor number of the old QEMU version. > +If you are still using machine types from QEMU versions older than 6 years, ditto > +please update your setting to use a newer versioned machine type instead. > + > Arm ``virt`` machine ``dtb-kaslr-seed`` property (since 7.1) > > Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [PULL 55/63] kvm: handle KVM_EXIT_MEMORY_FAULT
On Tue, 23 Apr 2024 at 16:16, Paolo Bonzini wrote:
>
> From: Chao Peng
>
> Upon an KVM_EXIT_MEMORY_FAULT exit, userspace needs to do the memory
> conversion on the RAMBlock to turn the memory into desired attribute,
> switching between private and shared.
>
> Currently only KVM_MEMORY_EXIT_FLAG_PRIVATE in flags is valid when
> KVM_EXIT_MEMORY_FAULT happens.
>
> Note, KVM_EXIT_MEMORY_FAULT makes sense only when the RAMBlock has
> guest_memfd memory backend.
>
> Note, KVM_EXIT_MEMORY_FAULT returns with -EFAULT, so special handling is
> added.
>
> When page is converted from shared to private, the original shared
> memory can be discarded via ram_block_discard_range(). Note, shared
> memory can be discarded only when it's not back'ed by hugetlb because
> hugetlb is supposed to be pre-allocated and no need for discarding.
>
> Signed-off-by: Chao Peng
> Co-developed-by: Xiaoyao Li
> Signed-off-by: Xiaoyao Li
>
> Message-ID: <20240320083945.991426-13-michael.r...@amd.com>
> Signed-off-by: Paolo Bonzini
Hi; Coverity points out an issue with this code (CID 1544114):
> +int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private)
> +{
> +MemoryRegionSection section;
> +ram_addr_t offset;
offset here is not initialized...
> +MemoryRegion *mr;
> +RAMBlock *rb;
> +void *addr;
> +int ret = -1;
> +
> +trace_kvm_convert_memory(start, size, to_private ? "shared_to_private" :
> "private_to_shared");
> +
> +if (!QEMU_PTR_IS_ALIGNED(start, qemu_real_host_page_size()) ||
> +!QEMU_PTR_IS_ALIGNED(size, qemu_real_host_page_size())) {
> +return -1;
> +}
> +
> +if (!size) {
> +return -1;
> +}
> +
> +section = memory_region_find(get_system_memory(), start, size);
> +mr = section.mr;
> +if (!mr) {
> +return -1;
> +}
> +
> +if (!memory_region_has_guest_memfd(mr)) {
> +error_report("Converting non guest_memfd backed memory region "
> + "(0x%"HWADDR_PRIx" ,+ 0x%"HWADDR_PRIx") to %s",
> + start, size, to_private ? "private" : "shared");
> +goto out_unref;
> +}
> +
> +if (to_private) {
> +ret = kvm_set_memory_attributes_private(start, size);
> +} else {
> +ret = kvm_set_memory_attributes_shared(start, size);
> +}
> +if (ret) {
> +goto out_unref;
> +}
> +
> +addr = memory_region_get_ram_ptr(mr) + section.offset_within_region;
> +rb = qemu_ram_block_from_host(addr, false, );
...and this call to qemu_ram_block_from_host() will only initialize
offset if it does not fail (i.e. doesn't return NULL)...
> +
> +if (to_private) {
> +if (rb->page_size != qemu_real_host_page_size()) {
...but here we assume rb is not NULL...
> +/*
> + * shared memory is backed by hugetlb, which is supposed to be
> + * pre-allocated and doesn't need to be discarded
> + */
> +goto out_unref;
> +}
> +ret = ram_block_discard_range(rb, offset, size);
> +} else {
> +ret = ram_block_discard_guest_memfd_range(rb, offset, size);
...and here we use offset assuming it has been initialized.
I think this code should either handle the case where
qemu_ram_block_from_host() fails, or, if it is impossible
for it to fail in this situation, add an assert() and a
comment about why we know it can't fail.
> +}
> +
> +out_unref:
> +memory_region_unref(mr);
> +return ret;
> +}
thanks
-- PMM
Re: [PATCH v2 3/4] hw/watchdog/sbsa_gwdt: Make watchdog timer frequency a QOM property
On Fri, 26 Apr 2024 at 13:46, Philippe Mathieu-Daudé wrote: > > Hi Peter, > > On 26/4/24 14:29, Peter Maydell wrote: > > Currently the sbsa_gdwt watchdog device hardcodes its frequency at > > 62.5MHz. In real hardware, this watchdog is supposed to be driven > > from the system counter, which also drives the CPU generic timers. > > Newer CPU types (in particular from Armv8.6) should have a CPU > > generic timer frequency of 1GHz, so we can't leave the watchdog > > on the old QEMU default of 62.5GHz. > > > > Make the frequency a QOM property so it can be set by the board, > > and have our only board that uses this device set that frequency > > to the same value it sets the CPU frequency. > > > > Signed-off-by: Peter Maydell > > --- > > include/hw/watchdog/sbsa_gwdt.h | 3 +-- > > hw/arm/sbsa-ref.c | 1 + > > hw/watchdog/sbsa_gwdt.c | 15 ++- > > 3 files changed, 16 insertions(+), 3 deletions(-) > > > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > > index 36f6f717b4b..57c337fd92a 100644 > > --- a/hw/arm/sbsa-ref.c > > +++ b/hw/arm/sbsa-ref.c > > @@ -543,6 +543,7 @@ static void create_wdt(const SBSAMachineState *sms) > > SysBusDevice *s = SYS_BUS_DEVICE(dev); > > int irq = sbsa_ref_irqmap[SBSA_GWDT_WS0]; > > > > +qdev_prop_set_uint64(dev, "clock-frequency", SBSA_GTIMER_HZ); > > Since we have access to the CPU and its generic timer, what about > just keep the wdg in sync, as smth like: > >qdev_prop_set_uint64(dev, "clock-frequency", > object_property_get_uint(OBJECT(some_cpu), > "cntfrq", errp)); That introduces an implicit ordering requirement that the CPU has been created before the watchdog, which I'm not super enthusiastic about. "The platform knows the frequency and sets it on the devices that care" seems more straightforward to me. (The really-follow-the-hardware approach here would be to model the memory mapped system counter and then wire that up to both the CPUs and the watchdog, but that's a lot of extra work. I have some half-baked patches in that direction but for the moment I figure doing the simple thing is all we need.) -- PMM
[PATCH v2 3/4] hw/watchdog/sbsa_gwdt: Make watchdog timer frequency a QOM property
Currently the sbsa_gdwt watchdog device hardcodes its frequency at
62.5MHz. In real hardware, this watchdog is supposed to be driven
from the system counter, which also drives the CPU generic timers.
Newer CPU types (in particular from Armv8.6) should have a CPU
generic timer frequency of 1GHz, so we can't leave the watchdog
on the old QEMU default of 62.5GHz.
Make the frequency a QOM property so it can be set by the board,
and have our only board that uses this device set that frequency
to the same value it sets the CPU frequency.
Signed-off-by: Peter Maydell
---
include/hw/watchdog/sbsa_gwdt.h | 3 +--
hw/arm/sbsa-ref.c | 1 +
hw/watchdog/sbsa_gwdt.c | 15 ++-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/include/hw/watchdog/sbsa_gwdt.h b/include/hw/watchdog/sbsa_gwdt.h
index 70b137de301..4bdc6c6fdb6 100644
--- a/include/hw/watchdog/sbsa_gwdt.h
+++ b/include/hw/watchdog/sbsa_gwdt.h
@@ -55,8 +55,6 @@
#define SBSA_GWDT_RMMIO_SIZE 0x1000
#define SBSA_GWDT_CMMIO_SIZE 0x1000
-#define SBSA_TIMER_FREQ 6250 /* Hz */
-
typedef struct SBSA_GWDTState {
/* */
SysBusDevice parent_obj;
@@ -67,6 +65,7 @@ typedef struct SBSA_GWDTState {
qemu_irq irq;
QEMUTimer *timer;
+uint64_t freq;
uint32_t id;
uint32_t wcs;
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 36f6f717b4b..57c337fd92a 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -543,6 +543,7 @@ static void create_wdt(const SBSAMachineState *sms)
SysBusDevice *s = SYS_BUS_DEVICE(dev);
int irq = sbsa_ref_irqmap[SBSA_GWDT_WS0];
+qdev_prop_set_uint64(dev, "clock-frequency", SBSA_GTIMER_HZ);
sysbus_realize_and_unref(s, _fatal);
sysbus_mmio_map(s, 0, rbase);
sysbus_mmio_map(s, 1, cbase);
diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index 96895d76369..d437535cc66 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -18,6 +18,7 @@
#include "qemu/osdep.h"
#include "sysemu/reset.h"
#include "sysemu/watchdog.h"
+#include "hw/qdev-properties.h"
#include "hw/watchdog/sbsa_gwdt.h"
#include "qemu/timer.h"
#include "migration/vmstate.h"
@@ -109,7 +110,7 @@ static void sbsa_gwdt_update_timer(SBSA_GWDTState *s,
WdtRefreshType rtype)
timeout = s->woru;
timeout <<= 32;
timeout |= s->worl;
-timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, SBSA_TIMER_FREQ);
+timeout = muldiv64(timeout, NANOSECONDS_PER_SECOND, s->freq);
timeout += qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
if ((rtype == EXPLICIT_REFRESH) || ((rtype == TIMEOUT_REFRESH) &&
@@ -261,6 +262,17 @@ static void wdt_sbsa_gwdt_realize(DeviceState *dev, Error
**errp)
dev);
}
+static Property wdt_sbsa_gwdt_props[] = {
+/*
+ * Timer frequency in Hz. This must match the frequency used by
+ * the CPU's generic timer. Default 62.5Hz matches QEMU's legacy
+ * CPU timer frequency default.
+ */
+DEFINE_PROP_UINT64("clock-frequency", struct SBSA_GWDTState, freq,
+ 6250),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static void wdt_sbsa_gwdt_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -271,6 +283,7 @@ static void wdt_sbsa_gwdt_class_init(ObjectClass *klass,
void *data)
set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
dc->vmsd = _sbsa_gwdt;
dc->desc = "SBSA-compliant generic watchdog device";
+device_class_set_props(dc, wdt_sbsa_gwdt_props);
}
static const TypeInfo wdt_sbsa_gwdt_info = {
--
2.34.1
Re: [PATCH] .gitlab-ci.d/cirrus: Remove the netbsd and openbsd jobs
On Fri, 26 Apr 2024 at 12:38, Thomas Huth wrote: > > During the past months, the netbsd and openbsd jobs in the Cirrus-CI > were broken most of the time - the setup to run a BSD in KVM on Cirrus-CI > from gitlab via the cirrus-run script was very fragile, and since the > jobs were not run by default, it used to bitrot very fast. > > Now Cirrus-CI also introduce a limit on the amount of free CI minutes > that you get there, so it is not appealing at all anymore to run > these BSDs in this setup - it's better to run the checks locally via > "make vm-build-openbsd" and "make vm-build-netbsd" instead. Thus let's > remove these CI jobs now. So what's the plan to keep BSD CI coverage? This seems like a step backwards towards "the person handling the pullreq merges has to do some local private ad-hoc testing too" :-( thanks -- PMM
[PATCH v2 4/4] target/arm: Default to 1GHz cntfrq for 'max' and new CPUs
In previous versions of the Arm architecture, the frequency of the
generic timers as reported in CNTFRQ_EL0 could be any IMPDEF value,
and for QEMU we picked 62.5MHz, giving a timer tick period of 16ns.
In Armv8.6, the architecture standardized this frequency to 1GHz.
Because there is no ID register feature field that indicates whether
a CPU is v8.6 or that it ought to have this counter frequency, we
implement this by changing our default CNTFRQ value for all CPUs,
with exceptions for backwards compatibility:
* CPU types which we already implement will retain the old
default value. None of these are v8.6 CPUs, so this is
architecturally OK.
* CPUs used in versioned machine types with a version of 9.0
or earlier will retain the old default value.
The upshot is that the only CPU type that changes is 'max'; but any
new type we add in future (whether v8.6 or not) will also get the new
1GHz default.
It remains the case that the machine model can override the default
value via the 'cntfrq' QOM property (regardless of the CPU type).
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
---
v1->v2: use DEFINE_PROP_BOOL in arm_cpu_properties[] instead of
qdev_property_add_static() to define backcompat-cntfrq property
---
target/arm/cpu.h | 11 +++
target/arm/internals.h | 12 ++--
hw/core/machine.c | 4 +++-
target/arm/cpu.c | 23 +--
target/arm/cpu64.c | 2 ++
target/arm/tcg/cpu32.c | 4
target/arm/tcg/cpu64.c | 18 ++
7 files changed, 65 insertions(+), 9 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 97997dbd087..b614bc5d139 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -959,6 +959,9 @@ struct ArchCPU {
*/
bool host_cpu_probe_failed;
+/* QOM property to indicate we should use the back-compat CNTFRQ default */
+bool backcompat_cntfrq;
+
/* Specify the number of cores in this CPU cluster. Used for the L2CTLR
* register.
*/
@@ -2359,6 +2362,14 @@ enum arm_features {
ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
ARM_FEATURE_M_MAIN, /* M profile Main Extension */
ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
+/*
+ * ARM_FEATURE_BACKCOMPAT_CNTFRQ makes the CPU default cntfrq be 62.5MHz
+ * if the board doesn't set a value, instead of 1GHz. It is for backwards
+ * compatibility and used only with CPU definitions that were already
+ * in QEMU before we changed the default. It should not be set on any
+ * CPU types added in future.
+ */
+ARM_FEATURE_BACKCOMPAT_CNTFRQ, /* 62.5MHz timer default */
};
static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index a1509a3a58e..5a5be347c67 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -61,9 +61,17 @@ static inline bool excp_is_internal(int excp)
/*
* Default frequency for the generic timer, in Hz.
- * This is 62.5MHz, which gives a 16 ns tick period.
+ * ARMv8.6 and later CPUs architecturally must use a 1GHz timer; before
+ * that it was an IMPDEF choice, and QEMU initially picked 62.5MHz,
+ * which gives a 16ns tick period.
+ *
+ * We will use the back-compat value:
+ * - for QEMU CPU types added before we standardized on 1GHz
+ * - for versioned machine types with a version of 9.0 or earlier
+ * In any case, the machine model may override via the cntfrq property.
*/
-#define GTIMER_DEFAULT_HZ 6250
+#define GTIMER_DEFAULT_HZ 10
+#define GTIMER_BACKCOMPAT_HZ 6250
/* Bit definitions for the v7M CONTROL register */
FIELD(V7M_CONTROL, NPRIV, 0, 1)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0dec48e8021..4ff60911e74 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -33,7 +33,9 @@
#include "hw/virtio/virtio-iommu.h"
#include "audio/audio.h"
-GlobalProperty hw_compat_9_0[] = {};
+GlobalProperty hw_compat_9_0[] = {
+{"arm-cpu", "backcompat-cntfrq", "true" },
+};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
GlobalProperty hw_compat_8_2[] = {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 9f2ca6633a1..fdc3eda318a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1959,13 +1959,22 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
if (!cpu->gt_cntfrq_hz) {
/*
- * 0 means "the board didn't set a value, use the default".
- * The default value of the generic timer frequency (as seen in
- * CNTFRQ_EL0) is 62.5MHz, which corresponds to a period of 16ns.
- * This is what you get (a) for a CONFIG_USER_ONLY CPU (b) if the
- * board doesn't set it.
+ * 0 means "the board didn't set a value, use the default". (We also
+ * get here for the CONFIG_USER_ONLY case.)
+
[PATCH v2 1/4] target/arm: Refactor default generic timer frequency handling
The generic timer frequency is settable by board code via a QOM
property "cntfrq", but otherwise defaults to 62.5MHz. The way this
is done includes some complication resulting from how this was
originally a fixed value with no QOM property. Clean it up:
* always set cpu->gt_cntfrq_hz to some sensible value, whether
the CPU has the generic timer or not, and whether it's system
or user-only emulation
* this means we can always use gt_cntfrq_hz, and never need
the old GTIMER_SCALE define
* set the default value in exactly one place, in the realize fn
The aim here is to pave the way for handling the ARMv8.6 requirement
that the generic timer frequency is always 1GHz. We're going to do
that by having old CPU types keep their legacy-in-QEMU behaviour and
having the default for any new CPU types be a 1GHz rather han 62.5MHz
cntfrq, so we want the point where the default is decided to be in
one place, and in code, not in a DEFINE_PROP_UINT64() initializer.
This commit should have no behavioural changes.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
---
target/arm/internals.h | 7 ---
target/arm/cpu.c | 31 +--
target/arm/helper.c| 16
3 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index b53f5e8ff2a..a1509a3a58e 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -59,10 +59,11 @@ static inline bool excp_is_internal(int excp)
|| excp == EXCP_SEMIHOST;
}
-/* Scale factor for generic timers, ie number of ns per tick.
- * This gives a 62.5MHz timer.
+/*
+ * Default frequency for the generic timer, in Hz.
+ * This is 62.5MHz, which gives a 16 ns tick period.
*/
-#define GTIMER_SCALE 16
+#define GTIMER_DEFAULT_HZ 6250
/* Bit definitions for the v7M CONTROL register */
FIELD(V7M_CONTROL, NPRIV, 0, 1)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a152def2413..9f2ca6633a1 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1506,9 +1506,12 @@ static void arm_cpu_initfn(Object *obj)
}
}
+/*
+ * 0 means "unset, use the default value". That default might vary depending
+ * on the CPU type, and is set in the realize fn.
+ */
static Property arm_cpu_gt_cntfrq_property =
-DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz,
- NANOSECONDS_PER_SECOND / GTIMER_SCALE);
+DEFINE_PROP_UINT64("cntfrq", ARMCPU, gt_cntfrq_hz, 0);
static Property arm_cpu_reset_cbar_property =
DEFINE_PROP_UINT64("reset-cbar", ARMCPU, reset_cbar, 0);
@@ -1954,6 +1957,17 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
return;
}
+if (!cpu->gt_cntfrq_hz) {
+/*
+ * 0 means "the board didn't set a value, use the default".
+ * The default value of the generic timer frequency (as seen in
+ * CNTFRQ_EL0) is 62.5MHz, which corresponds to a period of 16ns.
+ * This is what you get (a) for a CONFIG_USER_ONLY CPU (b) if the
+ * board doesn't set it.
+ */
+cpu->gt_cntfrq_hz = GTIMER_DEFAULT_HZ;
+}
+
#ifndef CONFIG_USER_ONLY
/* The NVIC and M-profile CPU are two halves of a single piece of
* hardware; trying to use one without the other is a command line
@@ -2002,18 +2016,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
}
{
-uint64_t scale;
-
-if (arm_feature(env, ARM_FEATURE_GENERIC_TIMER)) {
-if (!cpu->gt_cntfrq_hz) {
-error_setg(errp, "Invalid CNTFRQ: %"PRId64"Hz",
- cpu->gt_cntfrq_hz);
-return;
-}
-scale = gt_cntfrq_period_ns(cpu);
-} else {
-scale = GTIMER_SCALE;
-}
+uint64_t scale = gt_cntfrq_period_ns(cpu);
cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, scale,
arm_gt_ptimer_cb, cpu);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6b224826fbb..1e3002f9947 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2474,6 +2474,13 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
.resetvalue = 0 },
};
+static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+cpu->env.cp15.c14_cntfrq = cpu->gt_cntfrq_hz;
+}
+
#ifndef CONFIG_USER_ONLY
static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo
*ri,
@@ -3228,13 +3235,6 @@ void arm_gt_hvtimer_cb(void *opaque)
gt_recalc_timer(cpu, GTIMER_HYPVIRT);
}
-static void arm_gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
-{
-ARMCPU *cpu = env_archcpu(env);
-
-cpu->env.cp15.c14_cntfrq = cpu->
[PATCH v2 0/4] target/arm: Make the counter frequency default 1GHz for new CPUs, machines
In previous versions of the Arm architecture, the frequency of the generic timers as reported in CNTFRQ_EL0 could be any IMPDEF value, and for QEMU we picked 62.5MHz, giving a timer tick period of 16ns. In Armv8.6, the architecture standardized this frequency to 1GHz. Because there is no ID register feature field that indicates whether a CPU is v8.6 or that it ought to have this counter frequency, we implement this by changing our default CNTFRQ value for all CPUs, with exceptions for backwards compatibility: * CPU types which we already implement will retain the old default value. None of these are v8.6 CPUs, so this is architecturally OK. * CPUs used in versioned machine types with a version of 9.0 or earlier will retain the old default value. The upshot is that the only CPU type that changes is 'max'; but any new type we add in future (whether v8.6 or not) will also get the new 1GHz default (assuming we spot in code review any attempts to set the ARM_FEATURE_BACKCOMPAT_CNTFRQ flag on new CPU types as a result of cut-n-paste from an older CPU initfn ;-)). It remains the case that the machine model can override the default value via the 'cntfrq' QOM property (regardless of the CPU type). Unfortunately the TF-A firmware used to hard-code the CPU frequency, resulting in guest timers not running for the right duration. This is fixed in TF-A git but not yet in a release, and affects users running TF-A on either virt or sbsa-ref. For virt I think running TF-A is not a common setup, and besides we have versioned board models so users can use virt-9.0 if they want to run older TF-A binaries. For sbsa-ref the machine isn't versioned and TF-A is part of the standard guest software stack, so I've opted in this patchset to have our board model retain the old 62.5MHz clock for now. We can update that once e.g. TF-A has made a release with the fix (and we've updated our Avocado test's binaries!). I plan to leave it up to the sbsa-ref maintainers to decide when they're happy to make that change. Patches 1 and 4 are from v1 and have been reviewed. Patches 2 and 3 are new and together keep sbsa-ref on the old 62.5MHz value, at least for now. thanks -- PMM Peter Maydell (4): target/arm: Refactor default generic timer frequency handling hw/arm/sbsa-ref: Force CPU generic timer to 62.5MHz hw/watchdog/sbsa_gwdt: Make watchdog timer frequency a QOM property target/arm: Default to 1GHz cntfrq for 'max' and new CPUs include/hw/watchdog/sbsa_gwdt.h | 3 +-- target/arm/cpu.h| 11 + target/arm/internals.h | 15 +--- hw/arm/sbsa-ref.c | 16 + hw/core/machine.c | 4 +++- hw/watchdog/sbsa_gwdt.c | 15 +++- target/arm/cpu.c| 42 ++--- target/arm/cpu64.c | 2 ++ target/arm/helper.c | 16 ++--- target/arm/tcg/cpu32.c | 4 target/arm/tcg/cpu64.c | 18 ++ 11 files changed, 117 insertions(+), 29 deletions(-) -- 2.34.1
[PATCH v2 2/4] hw/arm/sbsa-ref: Force CPU generic timer to 62.5MHz
Currently QEMU CPUs always run with a generic timer counter frequency
of 62.5MHz, but ARMv8.6 CPUs will run at 1GHz. For older versions of
the TF-A firmware that sbsa-ref runs, the frequency of the generic
timer is hardcoded into the firmware, and so if the CPU actually has
a different frequency then timers in the guest will be set
incorrectly.
The default frequency used by the 'max' CPU is about to change, so
make the sbsa-ref board force the CPU frequency to the value which
the firmware expects.
Newer versions of TF-A will read the frequency from the CPU's
CNTFRQ_EL0 register:
https://github.com/ARM-software/arm-trusted-firmware/commit/4c77fac98dac0bebc63798aae9101ac865b87148
so in the longer term we could make this board use the 1GHz
frequency. We will need to make sure we update the binaries used
by our avocado test
Aarch64SbsarefMachine.test_sbsaref_alpine_linux_max_pauth_impdef
before we can do that.
Signed-off-by: Peter Maydell
---
I leave it up to the sbsa-ref maintainers exactly when they
want to shift to 1GHz (probably after a TF-A release with the fix?)
---
hw/arm/sbsa-ref.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f5709d6c141..36f6f717b4b 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -60,6 +60,19 @@
#define NUM_SMMU_IRQS 4
#define NUM_SATA_PORTS 6
+/*
+ * Generic timer frequency in Hz (which drives both the CPU generic timers
+ * and the SBSA watchdog-timer). Older versions of the TF-A firmware
+ * typically used with sbsa-ref (including the binaries in our Avocado test
+ * Aarch64SbsarefMachine.test_sbsaref_alpine_linux_max_pauth_impdef
+ * assume it is this value.
+ *
+ * TODO: this value is not architecturally correct for an Armv8.6 or
+ * better CPU, so we should move to 1GHz once the TF-A fix above has
+ * made it into a release and into our Avocado test.
+ */
+#define SBSA_GTIMER_HZ 6250
+
enum {
SBSA_FLASH,
SBSA_MEM,
@@ -767,6 +780,8 @@ static void sbsa_ref_init(MachineState *machine)
_abort);
}
+object_property_set_int(cpuobj, "cntfrq", SBSA_GTIMER_HZ,
_abort);
+
object_property_set_link(cpuobj, "memory", OBJECT(sysmem),
_abort);
--
2.34.1
Re: [PATCH] tests/avocado: update sunxi kernel from armbian to 6.6.16
Whoops, forgot to cc the allwinner maintainers/reviewers on this.
Ping for review, please?
thanks
-- PMM
On Mon, 15 Apr 2024 at 16:18, Peter Maydell wrote:
>
> The Linux kernel 5.10.16 binary for sunxi has been removed from
> apt.armbian.com. This means that the avocado tests for these machines
> will be skipped (status CANCEL) if the old binary isn't present in
> the avocado cache.
>
> Update to 6.6.16, in the same way we did in commit e384db41d8661
> when we moved to 5.10.16 in 2021.
>
> Cc: qemu-sta...@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2284
> Signed-off-by: Peter Maydell
> ---
> At this point in the release cycle I don't think I really want
> to put this into 9.0, though I could just about squeeze it in.
>
> cc'ing stable as an FYI -- since the tests fall back to the
> CANCEL status this doesn't break CI, so it's not a requirement
> to backport to any stable branches. But it would probably be
> preferable to get the coverage back on the stable branches so
> we can detect if we get something wrong on a backport of a
> patch that affects these machines.
> ---
> tests/avocado/boot_linux_console.py | 70 ++---
> tests/avocado/replay_kernel.py | 8 ++--
> 2 files changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/tests/avocado/boot_linux_console.py
> b/tests/avocado/boot_linux_console.py
> index 989b65111c0..d0ab5aaa83a 100644
> --- a/tests/avocado/boot_linux_console.py
> +++ b/tests/avocado/boot_linux_console.py
> @@ -646,12 +646,12 @@ def test_arm_cubieboard_initrd(self):
> :avocado: tags=accel:tcg
> """
> deb_url = ('https://apt.armbian.com/pool/main/l/'
> -
> 'linux-5.10.16-sunxi/linux-image-current-sunxi_21.02.2_armhf.deb')
> -deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
> +
> 'linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb')
> +deb_hash = 'f7c3c8c5432f765445dc6e7eab02f3bbe668256b'
> deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
> kernel_path = self.extract_from_deb(deb_path,
> -'/boot/vmlinuz-5.10.16-sunxi')
> -dtb_path =
> '/usr/lib/linux-image-current-sunxi/sun4i-a10-cubieboard.dtb'
> +
> '/boot/vmlinuz-6.6.16-current-sunxi')
> +dtb_path =
> '/usr/lib/linux-image-6.6.16-current-sunxi/sun4i-a10-cubieboard.dtb'
> dtb_path = self.extract_from_deb(deb_path, dtb_path)
> initrd_url = ('https://github.com/groeck/linux-build-test/raw/'
>'2eb0a73b5d5a28df3170c546ddaaa9757e1e0848/rootfs/'
> @@ -690,12 +690,12 @@ def test_arm_cubieboard_sata(self):
> :avocado: tags=accel:tcg
> """
> deb_url = ('https://apt.armbian.com/pool/main/l/'
> -
> 'linux-5.10.16-sunxi/linux-image-current-sunxi_21.02.2_armhf.deb')
> -deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
> +
> 'linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb')
> +deb_hash = 'f7c3c8c5432f765445dc6e7eab02f3bbe668256b'
> deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
> kernel_path = self.extract_from_deb(deb_path,
> -'/boot/vmlinuz-5.10.16-sunxi')
> -dtb_path =
> '/usr/lib/linux-image-current-sunxi/sun4i-a10-cubieboard.dtb'
> +
> '/boot/vmlinuz-6.6.16-current-sunxi')
> +dtb_path =
> '/usr/lib/linux-image-6.6.16-current-sunxi/sun4i-a10-cubieboard.dtb'
> dtb_path = self.extract_from_deb(deb_path, dtb_path)
> rootfs_url = ('https://github.com/groeck/linux-build-test/raw/'
>'2eb0a73b5d5a28df3170c546ddaaa9757e1e0848/rootfs/'
> @@ -872,13 +872,13 @@ def test_arm_bpim2u(self):
> :avocado: tags=machine:bpim2u
> :avocado: tags=accel:tcg
> """
> -deb_url = ('https://apt.armbian.com/pool/main/l/linux-5.10.16-sunxi/'
> - 'linux-image-current-sunxi_21.02.2_armhf.deb')
> -deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
> +deb_url = ('https://apt.armbian.com/pool/main/l/'
> +
> 'linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb')
> +deb_hash = 'f7c3c8c5432f765445dc6e7eab02f3bbe668256b'
> deb_path = self.fetch_asset(deb_url, asset_hash=deb_hash)
>
Re: [PATCH] fix bit fields extraction and prevent overflow
On Wed, 24 Apr 2024 at 19:13, Alexandra Diupina wrote: > > Add a type cast and use extract64() instead of extract32() > to avoid integer overflow on addition. Fix bit fields > extraction according to documentation. The commit message here says we make the handling of the address_extension fields match the documentation, and the version of the fixes to this function in the patch "[PATCH v2 RFC] fix host-endianness bug and prevent overflow" did that, but here we seem to have gone back to not changing the shift amounts. We also need to extract all 16 bits of the address_extension fields, not just 12, according to the datasheet. thanks -- PMM
Re: [PATCH] target/arm: Restrict translation disabled alignment check to VMSA
On Mon, 22 Apr 2024 at 18:07, Richard Henderson
wrote:
>
> For cpus using PMSA, when the MPU is disabled, the default memory
> type is Normal, Non-cachable.
>
> Fixes: 59754f85ed3 ("target/arm: Do memory type alignment check when
> translation disabled")
> Reported-by: Clément Chigot
> Signed-off-by: Richard Henderson
> ---
>
> Since v9 will likely be tagged tomorrow without this fixed,
> Cc: qemu-sta...@nongnu.org
Applied to target-arm.next (with the cc:stable added in and a couple of
trivial tweaks to the comment/commit message), thanks.
-- PMM
Re: [PATCH] hvf: arm: Remove PL1_WRITE_MASK
On Mon, 22 Apr 2024 at 10:27, Zenghui Yu wrote:
>
> As it had never been used since the first commit a1477da3ddeb ("hvf: Add
> Apple Silicon support").
>
> Signed-off-by: Zenghui Yu
> ---
> target/arm/hvf/hvf.c | 1 -
Applied to target-arm.next, thanks.
-- PMM
Re: [PATCH] hw/core/clock: remove assert in clock_propagate
On Fri, 19 Apr 2024 at 17:30, Raphael Poggi wrote: > > This commit allows childs clock to propagate their new frequency, > for example, after setting a new multiplier/diviser. > > Signed-off-by: Raphael Poggi Applied to target-arm.next, thanks. I rewrote the commit message to document the conversation we had in the other email thread: hw/core/clock: allow clock_propagate on child clocks clock_propagate() has an assert that clk->source is NULL, i.e. that you are calling it on a clock which has no source clock. This made sense in the original design where the only way for a clock's frequency to change if it had a source clock was when that source clock changed. However, we subsequently added multiplier/divider support, but didn't look at what that meant for propagation. If a clock-management device changes the multiplier or divider value on a clock, it needs to propagate that change down to child clocks, even if the clock has a source clock set. So the assertion is now incorrect. Remove the assertion. -- PMM
[PULL 02/37] target/arm: Add PSTATE.ALLINT
From: Jinjie Ruan
When PSTATE.ALLINT is set, an IRQ or FIQ interrupt that is targeted to
ELx, with or without superpriority is masked. As Richard suggested, place
ALLINT bit in PSTATE in env->pstate.
In the pseudocode, AArch64.ExceptionReturn() calls SetPSTATEFromPSR(), which
treats PSTATE.ALLINT as one of the bits which are reinstated from SPSR to
PSTATE regardless of whether this is an illegal exception return or not. So
handle PSTATE.ALLINT the same way as PSTATE.DAIF in the illegal_return exit
path of the exception_return helper. With the change, exception entry and
return are automatically handled.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-id: 20240407081733.3231820-3-ruanjin...@huawei.com
Signed-off-by: Peter Maydell
---
target/arm/cpu.h| 1 +
target/arm/tcg/helper-a64.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bc0c84873ff..de740d223fa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1430,6 +1430,7 @@ void pmu_init(ARMCPU *cpu);
#define PSTATE_D (1U << 9)
#define PSTATE_BTYPE (3U << 10)
#define PSTATE_SSBS (1U << 12)
+#define PSTATE_ALLINT (1U << 13)
#define PSTATE_IL (1U << 20)
#define PSTATE_SS (1U << 21)
#define PSTATE_PAN (1U << 22)
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ebaa7f00df3..29f3ef274ae 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -892,8 +892,8 @@ illegal_return:
*/
env->pstate |= PSTATE_IL;
env->pc = new_pc;
-spsr &= PSTATE_NZCV | PSTATE_DAIF;
-spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
+spsr &= PSTATE_NZCV | PSTATE_DAIF | PSTATE_ALLINT;
+spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF | PSTATE_ALLINT);
pstate_write(env, spsr);
if (!arm_singlestep_active(env)) {
env->pstate &= ~PSTATE_SS;
--
2.34.1
[PULL 14/37] hw/intc/arm_gicv3_kvm: Not set has-nmi=true for the KVM GICv3
From: Jinjie Ruan
So far, there is no FEAT_GICv3_NMI support in the in-kernel GIC, so make it
an error to try to set has-nmi=true for the KVM GICv3.
Signed-off-by: Jinjie Ruan
Message-id: 20240407081733.3231820-15-ruanjin...@huawei.com
Suggested-by: Peter Maydell
Signed-off-by: Peter Maydell
---
hw/intc/arm_gicv3_kvm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 77eb37e1317..00a383079b9 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -805,6 +805,11 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error
**errp)
return;
}
+if (s->nmi_support) {
+error_setg(errp, "NMI is not supported with the in-kernel GIC");
+return;
+}
+
gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
for (i = 0; i < s->num_cpu; i++) {
--
2.34.1
[PULL 13/37] hw/intc/arm_gicv3: Add has-nmi property to GICv3 device
From: Jinjie Ruan
Add a property has-nmi to the GICv3 device, and use this to set
the NMI bit in the GICD_TYPER register. This isn't visible to
guests yet because the property defaults to false and we won't
set it in the board code until we've landed all of the changes
needed to implement FEAT_GICV3_NMI.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-id: 20240407081733.3231820-14-ruanjin...@huawei.com
Signed-off-by: Peter Maydell
---
hw/intc/gicv3_internal.h | 1 +
include/hw/intc/arm_gicv3_common.h | 1 +
hw/intc/arm_gicv3_common.c | 1 +
hw/intc/arm_gicv3_dist.c | 2 ++
4 files changed, 5 insertions(+)
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 29d5cdc1b69..8f4ebed2f42 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -68,6 +68,7 @@
#define GICD_CTLR_E1NWF (1U << 7)
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_TYPER_NMI_SHIFT 9
#define GICD_TYPER_LPIS_SHIFT 17
/* 16 bits EventId */
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 7324c7d983f..4358c5319c1 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -249,6 +249,7 @@ struct GICv3State {
uint32_t num_irq;
uint32_t revision;
bool lpi_enable;
+bool nmi_support;
bool security_extn;
bool force_8bit_prio;
bool irq_reset_nonsecure;
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index c52f060026a..2d2cea6858a 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -569,6 +569,7 @@ static Property arm_gicv3_common_properties[] = {
DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
+DEFINE_PROP_BOOL("has-nmi", GICv3State, nmi_support, 0),
DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
/*
* Compatibility property: force 8 bits of physical priority, even
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index 35e850685c0..22ddc0d6661 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -389,6 +389,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
* by GICD_TYPER.IDbits)
* MBIS == 0 (message-based SPIs not supported)
* SecurityExtn == 1 if security extns supported
+ * NMI = 1 if Non-maskable interrupt property is supported
* CPUNumber == 0 since for us ARE is always 1
* ITLinesNumber == (((max SPI IntID + 1) / 32) - 1)
*/
@@ -402,6 +403,7 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
bool dvis = s->revision >= 4;
*data = (1 << 25) | (1 << 24) | (dvis << 18) | (sec_extn << 10) |
+(s->nmi_support << GICD_TYPER_NMI_SHIFT) |
(s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
(0xf << 19) | itlinesnumber;
return true;
--
2.34.1
[PULL 03/37] target/arm: Add support for FEAT_NMI, Non-maskable Interrupt
From: Jinjie Ruan
Add support for FEAT_NMI. NMI (FEAT_NMI) is an mandatory feature in
ARMv8.8-A and ARM v9.3-A.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-id: 20240407081733.3231820-4-ruanjin...@huawei.com
Signed-off-by: Peter Maydell
---
target/arm/internals.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index dd3da211a3f..516e0584bf5 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1229,6 +1229,9 @@ static inline uint32_t aarch64_pstate_valid_mask(const
ARMISARegisters *id)
if (isar_feature_aa64_mte(id)) {
valid |= PSTATE_TCO;
}
+if (isar_feature_aa64_nmi(id)) {
+valid |= PSTATE_ALLINT;
+}
return valid;
}
--
2.34.1
[PULL 04/37] target/arm: Implement ALLINT MSR (immediate)
From: Jinjie Ruan
Add ALLINT MSR (immediate) to decodetree, in which the CRm is 0b000x. The
EL0 check is necessary to ALLINT, and the EL1 check is necessary when
imm == 1. So implement it inline for EL2/3, or EL1 with imm==0. Avoid the
unconditional write to pc and use raise_exception_ra to unwind.
Signed-off-by: Jinjie Ruan
Reviewed-by: Richard Henderson
Reviewed-by: Peter Maydell
Message-id: 20240407081733.3231820-5-ruanjin...@huawei.com
Signed-off-by: Peter Maydell
---
target/arm/tcg/helper-a64.h| 1 +
target/arm/tcg/a64.decode | 1 +
target/arm/tcg/helper-a64.c| 12
target/arm/tcg/translate-a64.c | 19 +++
4 files changed, 33 insertions(+)
diff --git a/target/arm/tcg/helper-a64.h b/target/arm/tcg/helper-a64.h
index 575a5dab7dc..05181653999 100644
--- a/target/arm/tcg/helper-a64.h
+++ b/target/arm/tcg/helper-a64.h
@@ -22,6 +22,7 @@ DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
DEF_HELPER_2(msr_i_spsel, void, env, i32)
DEF_HELPER_2(msr_i_daifset, void, env, i32)
DEF_HELPER_2(msr_i_daifclear, void, env, i32)
+DEF_HELPER_1(msr_set_allint_el1, void, env)
DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr)
DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr)
DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 8a20dce3c8f..0e7656fd158 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -207,6 +207,7 @@ MSR_i_DIT 1101 0101 0 011 0100 010 1
@msr_i
MSR_i_TCO 1101 0101 0 011 0100 100 1 @msr_i
MSR_i_DAIFSET 1101 0101 0 011 0100 110 1 @msr_i
MSR_i_DAIFCLEAR 1101 0101 0 011 0100 111 1 @msr_i
+MSR_i_ALLINT1101 0101 0 001 0100 000 imm:1 000 1
MSR_i_SVCR 1101 0101 0 011 0100 0 mask:2 imm:1 011 1
# MRS, MSR (register), SYS, SYSL. These are all essentially the
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index 29f3ef274ae..0ea8668ab4c 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -66,6 +66,18 @@ void HELPER(msr_i_spsel)(CPUARMState *env, uint32_t imm)
update_spsel(env, imm);
}
+void HELPER(msr_set_allint_el1)(CPUARMState *env)
+{
+/* ALLINT update to PSTATE. */
+if (arm_hcrx_el2_eff(env) & HCRX_TALLINT) {
+raise_exception_ra(env, EXCP_UDEF,
+ syn_aa64_sysregtrap(0, 1, 0, 4, 1, 0x1f, 0), 2,
+ GETPC());
+}
+
+env->pstate |= PSTATE_ALLINT;
+}
+
static void daif_check(CPUARMState *env, uint32_t op,
uint32_t imm, uintptr_t ra)
{
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 2666d527111..976094a5c80 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2036,6 +2036,25 @@ static bool trans_MSR_i_DAIFCLEAR(DisasContext *s, arg_i
*a)
return true;
}
+static bool trans_MSR_i_ALLINT(DisasContext *s, arg_i *a)
+{
+if (!dc_isar_feature(aa64_nmi, s) || s->current_el == 0) {
+return false;
+}
+
+if (a->imm == 0) {
+clear_pstate_bits(PSTATE_ALLINT);
+} else if (s->current_el > 1) {
+set_pstate_bits(PSTATE_ALLINT);
+} else {
+gen_helper_msr_set_allint_el1(tcg_env);
+}
+
+/* Exit the cpu loop to re-evaluate pending IRQs. */
+s->base.is_jmp = DISAS_UPDATE_EXIT;
+return true;
+}
+
static bool trans_MSR_i_SVCR(DisasContext *s, arg_MSR_i_SVCR *a)
{
if (!dc_isar_feature(aa64_sme, s) || a->mask == 0) {
--
2.34.1
