Re: [PATCH v9 4/5] KVM: arm/arm64: remove pmc->bitmask
Hi Andrew,
On 12/06/2019 20:04, Andrew Murray wrote:
> We currently use pmc->bitmask to determine the width of the pmc - however
> it's superfluous as the pmc index already describes if the pmc is a cycle
> counter or event counter. The architecture clearly describes the widths of
> these counters.
>
> Let's remove the bitmask to simplify the code.
>
> Signed-off-by: Andrew Murray
> ---
> include/kvm/arm_pmu.h | 1 -
> virt/kvm/arm/pmu.c| 19 +--
> 2 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index b73f31baca52..2f0e28dc5a9e 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -28,7 +28,6 @@
> struct kvm_pmc {
> u8 idx; /* index into the pmu->pmc array */
> struct perf_event *perf_event;
> - u64 bitmask;
> };
>
> struct kvm_pmu {
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index ae1e886d4a1a..88ce24ae0b45 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -47,7 +47,10 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64
> select_idx)
> counter += perf_event_read_value(pmc->perf_event, &enabled,
>&running);
>
> - return counter & pmc->bitmask;
> + if (select_idx != ARMV8_PMU_CYCLE_IDX)
> + counter = lower_32_bits(counter);
Shouldn't this depend on PMCR.LC as well? If PMCR.LC is clear we only
want the lower 32bits of the cycle counter.
Cheers,
--
Julien Thierry
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v9 4/5] KVM: arm/arm64: remove pmc->bitmask
On Thu, Jun 13, 2019 at 08:30:51AM +0100, Julien Thierry wrote:
> Hi Andrew,
>
> On 12/06/2019 20:04, Andrew Murray wrote:
> > We currently use pmc->bitmask to determine the width of the pmc - however
> > it's superfluous as the pmc index already describes if the pmc is a cycle
> > counter or event counter. The architecture clearly describes the widths of
> > these counters.
> >
> > Let's remove the bitmask to simplify the code.
> >
> > Signed-off-by: Andrew Murray
> > ---
> > include/kvm/arm_pmu.h | 1 -
> > virt/kvm/arm/pmu.c| 19 +--
> > 2 files changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> > index b73f31baca52..2f0e28dc5a9e 100644
> > --- a/include/kvm/arm_pmu.h
> > +++ b/include/kvm/arm_pmu.h
> > @@ -28,7 +28,6 @@
> > struct kvm_pmc {
> > u8 idx; /* index into the pmu->pmc array */
> > struct perf_event *perf_event;
> > - u64 bitmask;
> > };
> >
> > struct kvm_pmu {
> > diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> > index ae1e886d4a1a..88ce24ae0b45 100644
> > --- a/virt/kvm/arm/pmu.c
> > +++ b/virt/kvm/arm/pmu.c
> > @@ -47,7 +47,10 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64
> > select_idx)
> > counter += perf_event_read_value(pmc->perf_event, &enabled,
> > &running);
> >
> > - return counter & pmc->bitmask;
> > + if (select_idx != ARMV8_PMU_CYCLE_IDX)
> > + counter = lower_32_bits(counter);
>
> Shouldn't this depend on PMCR.LC as well? If PMCR.LC is clear we only
> want the lower 32bits of the cycle counter.
Yes that's correct. The hunk should look like this:
- return counter & pmc->bitmask;
+ if (!(select_idx == ARMV8_PMU_CYCLE_IDX &&
+ __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC))
+ counter = lower_32_bits(counter);
+
+ return counter;
Thanks for the review.
Andrew Murray
>
> Cheers,
>
> --
> Julien Thierry
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] KVM: arm/arm64: fix emulated ptimer irq injection
On Mon, 03 Jun 2019 13:14:40 +0100,
Andrew Jones wrote:
>
> On Wed, May 29, 2019 at 12:03:11PM +0200, Christoffer Dall wrote:
> > On Wed, May 29, 2019 at 10:13:21AM +0100, Marc Zyngier wrote:
> > > On 29/05/2019 10:08, Christoffer Dall wrote:
> > > > On Tue, May 28, 2019 at 05:08:53PM +0100, Marc Zyngier wrote:
> > > >> On 28/05/2019 14:40, Andrew Jones wrote:
> > > >>> On Tue, May 28, 2019 at 03:12:15PM +0200, Christoffer Dall wrote:
> > > On Tue, May 28, 2019 at 01:25:52PM +0100, Marc Zyngier wrote:
> > > > On 28/05/2019 12:01, Christoffer Dall wrote:
> > > >> On Mon, May 27, 2019 at 01:46:19PM +0200, Andrew Jones wrote:
> > > >>> The emulated ptimer needs to track the level changes, otherwise
> > > >>> the
> > > >>> the interrupt will never get deasserted, resulting in the guest
> > > >>> getting
> > > >>> stuck in an interrupt storm if it enables ptimer interrupts. This
> > > >>> was
> > > >>> found with kvm-unit-tests; the ptimer tests hung as soon as
> > > >>> interrupts
> > > >>> were enabled. Typical Linux guests don't have a problem as they
> > > >>> prefer
> > > >>> using the virtual timer.
> > > >>>
> > > >>> Fixes: bee038a674875 ("KVM: arm/arm64: Rework the timer code to
> > > >>> use a timer_map")
> > > >>> Signed-off-by: Andrew Jones
> > > >>> ---
> > > >>> virt/kvm/arm/arch_timer.c | 7 ++-
> > > >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> > > >>> index 7fc272ecae16..9f5d8cc8b5e5 100644
> > > >>> --- a/virt/kvm/arm/arch_timer.c
> > > >>> +++ b/virt/kvm/arm/arch_timer.c
> > > >>> @@ -324,10 +324,15 @@ static void kvm_timer_update_irq(struct
> > > >>> kvm_vcpu *vcpu, bool new_level,
> > > >>> static void timer_emulate(struct arch_timer_context *ctx)
> > > >>> {
> > > >>> bool should_fire = kvm_timer_should_fire(ctx);
> > > >>> + struct timer_map map;
> > > >>> +
> > > >>> + get_timer_map(ctx->vcpu, &map);
> > > >>>
> > > >>> trace_kvm_timer_emulate(ctx, should_fire);
> > > >>>
> > > >>> - if (should_fire) {
> > > >>> + if (ctx == map.emul_ptimer && should_fire != ctx->irq.level) {
> > > >>> + kvm_timer_update_irq(ctx->vcpu, !ctx->irq.level, ctx);
> > > >>> + } else if (should_fire) {
> > > >>> kvm_timer_update_irq(ctx->vcpu, true, ctx);
> > > >>> return;
> > > >>> }
> > > >>
> > > >> Hmm, this doesn't feel completely right.
> > > >>>
> > > >>> I won't try to argue that this is the right fix, as I haven't fully
> > > >>> grasped how all this code works, but, afaict, this is how it worked
> > > >>> prior to bee038a6.
> > > >>>
> > > >>
> > > >> Lowering the line of an emulated timer should only ever happen
> > > >> when the
> > > >> guest (or user space) writes to one of the system registers for
> > > >> that
> > > >> timer, which should be trapped and that should cause an update of
> > > >> the
> > > >> line.
> > > >>
> > > >> Are we missing a call to kvm_timer_update_irq() from
> > > >> kvm_arm_timer_set_reg() ?
> > > >
> > > > Which is exactly what we removed in 6bc210003dff, for good reasons.
> > > >
> > >
> > > Ah well, I can be wrong twice. Or even three times.
> > >
> > > > Looking at kvm_arm_timer_write_sysreg(), we end-up calling
> > > > kvm_timer_vcpu_load, but not updating the irq status.
> > > >
> > > > How about something like this instead (untested):
> > > >
> > > > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> > > > index 7fc272ecae16..6a418dcc5433 100644
> > > > --- a/virt/kvm/arm/arch_timer.c
> > > > +++ b/virt/kvm/arm/arch_timer.c
> > > > @@ -882,10 +882,14 @@ void kvm_arm_timer_write_sysreg(struct
> > > > kvm_vcpu *vcpu,
> > > > enum kvm_arch_timer_regs treg,
> > > > u64 val)
> > > > {
> > > > + struct arch_timer_context *timer;
> > > > +
> > > > preempt_disable();
> > > > kvm_timer_vcpu_put(vcpu);
> > > >
> > > > - kvm_arm_timer_write(vcpu, vcpu_get_timer(vcpu, tmr), treg, val);
> > > > + timer = vcpu_get_timer(vcpu, tmr);
> > > > + kvm_arm_timer_write(vcpu, timer, treg, val);
> > > > + kvm_timer_update_irq(vcpu, kvm_timer_should_fire(timer), timer);
> > > >
> > > > kvm_timer_vcpu_load(vcpu);
> > > > preempt_enable();
> > > >
> > > >>>
> > > >>> Marc, I've tested this and it resolves the issue for me. If/when you
> > > >>> post
> > > >>> it you can add a t-b from me if you like.
> > > >>>
> > >
> > > Yes, that looks reasonable. Basically, in 6bc210003dff we should
> > > have
> > > only removed the call to timer_emulate, and not messed
Re: [PATCH v1 1/5] KVM: arm/arm64: Remove kvm_mmio_emulate tracepoint
Hi James,
On 2019/6/12 20:48, James Morse wrote:
Hi,
On 12/06/2019 10:08, Zenghui Yu wrote:
In current KVM/ARM code, no one will invoke trace_kvm_mmio_emulate().
Remove this TRACE_EVENT definition.
Oooer. We can't just go removing these things, they are visible to user-space.
I recall an article on this: https://lwn.net/Articles/737530/
"Another attempt to address the tracepoint ABI problem"
I agree this is orphaned, it was added by commit 45e96ea6b369 ("KVM: ARM:
Handle I/O
aborts"), but there never was a caller.
The problem with removing it is
/sys/kernel/debug/tracing/events/kvm/kvm_mmio_emulate
disappears. Any program relying on that being present (but useless) is now
broken.
Thanks for the reminder.
It turned out that I knew little about the tracepoint ABI :( .
I'm OK to just drop this patch in next version.
Thanks,
zenghui
.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v1 2/5] KVM: arm/arm64: Adjust entry/exit and trap related tracepoints
Hi James,
On 2019/6/12 20:49, James Morse wrote:
Hi,
On 12/06/2019 10:08, Zenghui Yu wrote:
Currently, we use trace_kvm_exit() to report exception type (e.g.,
"IRQ", "TRAP") and exception class (ESR_ELx's bit[31:26]) together.
(They both caused an exit!)
But hardware only saves the exit class to ESR_ELx on synchronous
EC is the 'Exception Class'. Exit is KVM/Linux's terminology.
Yes, a stupid mistake ;-)
exceptions, not on asynchronous exceptions. When the guest exits
due to external interrupts, we will get tracing output like:
"kvm_exit: IRQ: HSR_EC: 0x (UNKNOWN), PC: 0x87259e30"
Obviously, "HSR_EC" here is meaningless.
I assume we do it this way so there is only one guest-exit tracepoint that
catches all exits.
I don't think its a problem if user-space has to know the EC isn't set for
asynchronous
exceptions, this is a property of the architecture and anything using these
trace-points
is already arch specific.
Actually, *no* problem in current implementation, and I'm OK to still
keep the EC in trace_kvm_exit(). What I really want to do is adding the
EC in trace_trap_enter (the new tracepoint), will explain it later.
This patch splits "exit" and "trap" events by adding two tracepoints
explicitly in handle_trap_exceptions(). Let trace_kvm_exit() report VM
exit events, and trace_kvm_trap_exit() report VM trap events.
These tracepoints are adjusted also in preparation for supporting
'perf kvm stat' on arm64.
Because the existing tracepoints are ABI, I don't think we can change them.
We can add new ones if there is something that a user reasonably needs to
trace, and can't
be done any other way.
What can't 'perf kvm stat' do with the existing trace points?
(A good question! I should have made it clear in the commit message,
forgive me.)
First, how does 'perf kvm stat' interact with tracepoints?
We have three handlers for a specific event (e.g., "VM-EXIT") --
"is_begin_event", "is_end_event", "decode_key". The first two handlers
make use of two existing tracepoints ("kvm:kvm_exit" & "kvm:kvm_entry")
to check when the VM-EXIT events started/ended, thus the time difference
stats, event start/end time etc. can be calculated.
"is_begin_event" handler gets a *key* from the "ret" field (exit_code)
of "kvm:kvm_exit" payload, and "decode_key" handler makes use of the
*key* to find out the reason for the VM-EXIT event. Of course we should
maintain the mapping between exit_code and exit_reason in userspace.
These are all what *patch #4* had done, #4 is a simple patch to review!
Oh, we can also set "vcpu_id_str" to achieve per vcpu event record, but
currently, we only have the "vcpu_pc" field in "kvm:kvm_entry", without
something like "vcpu_id".
perf people must have a much deeper understanding of this.
OK, next comes the more important question - what should/can we do to
the tracepoints in preparation of 'perf kvm stat' on arm64?
From the article you've provided, it's clear that we can't remove the EC
from trace_kvm_exit(). But can we add something like "vcpu_id" into
(at least) trace_kvm_entry(), just like what this patch has done?
If not, which means we have to keep the existing tracepoints totally
unchanged, then 'perf kvm stat' will have no way to record/report per
vcpu VM-EXIT events (other arch like X86, powerpc, s390 etc. have this
capability, if I understand it correctly).
As for TRAP events, should we consider adding two new tracepoints --
"kvm_trap_enter" and "kvm_trap_exit", to keep tracking of the trap
handling process? We should also record the EC in "kvm_trap_enter",
which will be used as *key* in TRAP event's "is_begin_event" handler.
Patch #5 tells us the whole story, it's simple too.
What do you suggest?
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 516aead..af3c732 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -264,7 +264,10 @@ static int handle_trap_exceptions(struct kvm_vcpu *vcpu,
struct kvm_run *run)
exit_handle_fn exit_handler;
exit_handler = kvm_get_exit_handler(vcpu);
+ trace_kvm_trap_enter(vcpu->vcpu_id,
+kvm_vcpu_trap_get_class(vcpu));
handled = exit_handler(vcpu, run);
+ trace_kvm_trap_exit(vcpu->vcpu_id);
}
Why are there two? Are you using this to benchmark the exit_handler()?
Almostly yes. Let perf know when the TRAP handling event start/end,
and ...
As we can't remove the EC from the exit event, I don't think this tells us
anything new.
As explained above, this EC is for 'perf kvm stat'.
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 90cedeb..9f63fd9 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -758,7 +758,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct
kvm_run *run)
/**
* Enter the guest
*/
-
Re: [PATCH] KVM: arm/arm64: fix emulated ptimer irq injection
On Thu, Jun 13, 2019 at 11:01:41AM +0100, Marc Zyngier wrote:
> On Mon, 03 Jun 2019 13:14:40 +0100,
> Andrew Jones wrote:
> >
> > On Wed, May 29, 2019 at 12:03:11PM +0200, Christoffer Dall wrote:
> > > On Wed, May 29, 2019 at 10:13:21AM +0100, Marc Zyngier wrote:
> > > > On 29/05/2019 10:08, Christoffer Dall wrote:
> > > > > On Tue, May 28, 2019 at 05:08:53PM +0100, Marc Zyngier wrote:
> > > > >> On 28/05/2019 14:40, Andrew Jones wrote:
> > > > >>> On Tue, May 28, 2019 at 03:12:15PM +0200, Christoffer Dall wrote:
> > > > On Tue, May 28, 2019 at 01:25:52PM +0100, Marc Zyngier wrote:
> > > > > On 28/05/2019 12:01, Christoffer Dall wrote:
> > > > >> On Mon, May 27, 2019 at 01:46:19PM +0200, Andrew Jones wrote:
> > > > >>> The emulated ptimer needs to track the level changes, otherwise
> > > > >>> the
> > > > >>> the interrupt will never get deasserted, resulting in the guest
> > > > >>> getting
> > > > >>> stuck in an interrupt storm if it enables ptimer interrupts.
> > > > >>> This was
> > > > >>> found with kvm-unit-tests; the ptimer tests hung as soon as
> > > > >>> interrupts
> > > > >>> were enabled. Typical Linux guests don't have a problem as they
> > > > >>> prefer
> > > > >>> using the virtual timer.
> > > > >>>
> > > > >>> Fixes: bee038a674875 ("KVM: arm/arm64: Rework the timer code to
> > > > >>> use a timer_map")
> > > > >>> Signed-off-by: Andrew Jones
> > > > >>> ---
> > > > >>> virt/kvm/arm/arch_timer.c | 7 ++-
> > > > >>> 1 file changed, 6 insertions(+), 1 deletion(-)
> > > > >>>
> > > > >>> diff --git a/virt/kvm/arm/arch_timer.c
> > > > >>> b/virt/kvm/arm/arch_timer.c
> > > > >>> index 7fc272ecae16..9f5d8cc8b5e5 100644
> > > > >>> --- a/virt/kvm/arm/arch_timer.c
> > > > >>> +++ b/virt/kvm/arm/arch_timer.c
> > > > >>> @@ -324,10 +324,15 @@ static void kvm_timer_update_irq(struct
> > > > >>> kvm_vcpu *vcpu, bool new_level,
> > > > >>> static void timer_emulate(struct arch_timer_context *ctx)
> > > > >>> {
> > > > >>> bool should_fire = kvm_timer_should_fire(ctx);
> > > > >>> + struct timer_map map;
> > > > >>> +
> > > > >>> + get_timer_map(ctx->vcpu, &map);
> > > > >>>
> > > > >>> trace_kvm_timer_emulate(ctx, should_fire);
> > > > >>>
> > > > >>> - if (should_fire) {
> > > > >>> + if (ctx == map.emul_ptimer && should_fire !=
> > > > >>> ctx->irq.level) {
> > > > >>> + kvm_timer_update_irq(ctx->vcpu,
> > > > >>> !ctx->irq.level, ctx);
> > > > >>> + } else if (should_fire) {
> > > > >>> kvm_timer_update_irq(ctx->vcpu, true, ctx);
> > > > >>> return;
> > > > >>> }
> > > > >>
> > > > >> Hmm, this doesn't feel completely right.
> > > > >>>
> > > > >>> I won't try to argue that this is the right fix, as I haven't fully
> > > > >>> grasped how all this code works, but, afaict, this is how it worked
> > > > >>> prior to bee038a6.
> > > > >>>
> > > > >>
> > > > >> Lowering the line of an emulated timer should only ever happen
> > > > >> when the
> > > > >> guest (or user space) writes to one of the system registers for
> > > > >> that
> > > > >> timer, which should be trapped and that should cause an update
> > > > >> of the
> > > > >> line.
> > > > >>
> > > > >> Are we missing a call to kvm_timer_update_irq() from
> > > > >> kvm_arm_timer_set_reg() ?
> > > > >
> > > > > Which is exactly what we removed in 6bc210003dff, for good
> > > > > reasons.
> > > > >
> > > >
> > > > Ah well, I can be wrong twice. Or even three times.
> > > >
> > > > > Looking at kvm_arm_timer_write_sysreg(), we end-up calling
> > > > > kvm_timer_vcpu_load, but not updating the irq status.
> > > > >
> > > > > How about something like this instead (untested):
> > > > >
> > > > > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> > > > > index 7fc272ecae16..6a418dcc5433 100644
> > > > > --- a/virt/kvm/arm/arch_timer.c
> > > > > +++ b/virt/kvm/arm/arch_timer.c
> > > > > @@ -882,10 +882,14 @@ void kvm_arm_timer_write_sysreg(struct
> > > > > kvm_vcpu *vcpu,
> > > > > enum kvm_arch_timer_regs treg,
> > > > > u64 val)
> > > > > {
> > > > > + struct arch_timer_context *timer;
> > > > > +
> > > > > preempt_disable();
> > > > > kvm_timer_vcpu_put(vcpu);
> > > > >
> > > > > - kvm_arm_timer_write(vcpu, vcpu_get_timer(vcpu, tmr), treg, val);
> > > > > + timer = vcpu_get_timer(vcpu, tmr);
> > > > > + kvm_arm_timer_write(vcpu, timer, treg, val);
> > > > > + kvm_timer_update_irq(vcpu, kvm_timer_should_fire(timer), timer);
> > > > >
> > > > > kvm_timer_vcpu_load(vcpu);
> > >
Re: [PATCH v8 0/7] Add virtio-iommu driver
Hi, On 5/30/19 7:09 PM, Jean-Philippe Brucker wrote: > Implement the virtio-iommu driver, following specification v0.12 [1]. > Since last version [2] we've worked on improving the specification, > which resulted in the following changes to the interface: > * Remove the EXEC flag. > * Add feature bit for the MMIO flag. > * Change domain_bits to domain_range. > > Given that there were small changes to patch 5/7, I removed the review > and test tags. Please find the code at [3]. > > [1] Virtio-iommu specification v0.12, sources and pdf > git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.12 > http://jpbrucker.net/virtio-iommu/spec/v0.12/virtio-iommu-v0.12.pdf > > http://jpbrucker.net/virtio-iommu/spec/diffs/virtio-iommu-dev-diff-v0.11-v0.12.pdf > > [2] [PATCH v7 0/7] Add virtio-iommu driver > > https://lore.kernel.org/linux-pci/0ba215f5-e856-bf31-8dd9-a85710714...@arm.com/T/ > > [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.12 > git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.12 SERIES Tested-by: Eric Auger with QEMU branch: https://github.com/eauger/qemu/tree/v4.0-virtio-iommu-v0.12 and kernel branch: https://github.com/eauger/linux/tree/virtio-iommu-v0.12 (where I just added the rebased iort patches on top of Jean-Philippe's virtio-iommu/v0.12 branch). So I also tested the ACPI boot. I will formally submit the QEMU series ASAP. Thanks Eric > > Jean-Philippe Brucker (7): > dt-bindings: virtio-mmio: Add IOMMU description > dt-bindings: virtio: Add virtio-pci-iommu node > of: Allow the iommu-map property to omit untranslated devices > PCI: OF: Initialize dev->fwnode appropriately > iommu: Add virtio-iommu driver > iommu/virtio: Add probe request > iommu/virtio: Add event queue > > .../devicetree/bindings/virtio/iommu.txt | 66 + > .../devicetree/bindings/virtio/mmio.txt | 30 + > MAINTAINERS |7 + > drivers/iommu/Kconfig | 11 + > drivers/iommu/Makefile|1 + > drivers/iommu/virtio-iommu.c | 1176 + > drivers/of/base.c | 10 +- > drivers/pci/of.c |6 + > include/uapi/linux/virtio_ids.h |1 + > include/uapi/linux/virtio_iommu.h | 165 +++ > 10 files changed, 1470 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt > create mode 100644 drivers/iommu/virtio-iommu.c > create mode 100644 include/uapi/linux/virtio_iommu.h > ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v9 4/5] KVM: arm/arm64: remove pmc->bitmask
On 13/06/2019 10:39, Andrew Murray wrote:
On Thu, Jun 13, 2019 at 08:30:51AM +0100, Julien Thierry wrote:
Hi Andrew,
On 12/06/2019 20:04, Andrew Murray wrote:
We currently use pmc->bitmask to determine the width of the pmc - however
it's superfluous as the pmc index already describes if the pmc is a cycle
counter or event counter. The architecture clearly describes the widths of
these counters.
Let's remove the bitmask to simplify the code.
Signed-off-by: Andrew Murray
---
include/kvm/arm_pmu.h | 1 -
virt/kvm/arm/pmu.c| 19 +--
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index b73f31baca52..2f0e28dc5a9e 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -28,7 +28,6 @@
struct kvm_pmc {
u8 idx; /* index into the pmu->pmc array */
struct perf_event *perf_event;
- u64 bitmask;
};
struct kvm_pmu {
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index ae1e886d4a1a..88ce24ae0b45 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -47,7 +47,10 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64
select_idx)
counter += perf_event_read_value(pmc->perf_event, &enabled,
&running);
- return counter & pmc->bitmask;
+ if (select_idx != ARMV8_PMU_CYCLE_IDX)
+ counter = lower_32_bits(counter);
Shouldn't this depend on PMCR.LC as well? If PMCR.LC is clear we only
want the lower 32bits of the cycle counter.
Yes that's correct. The hunk should look like this:
- return counter & pmc->bitmask;
+ if (!(select_idx == ARMV8_PMU_CYCLE_IDX &&
+ __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC))
+ counter = lower_32_bits(counter);
+
+ return counter;
May be you could add a macro :
#define vcpu_pmu_counter_is_64bit(vcpu, idx) ?
Cheers
Suzuki
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
