* Re: [PATCH v14 01/12] powerpc/32: Fix CONFIG_VIRT_CPU_ACCOUNTING_NATIVE for 40x/booke
From: Christophe Leroy @ 2019-01-30 8:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin, Mike Rapoport
Cc: Mark Rutland, linuxppc-dev, linux-kernel
In-Reply-To: <90ef676a0fba977259221d95dd8ffd1117f963f3.1548346225.git.christophe.leroy@c-s.fr>
Le 24/01/2019 à 17:19, Christophe Leroy a écrit :
> 40x/booke have another path to reach 3f from transfer_to_handler,
> so ACCOUNT_CPU_USER_ENTRY() have to be moved there.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Now ACCOUNT_CPU_USER_ENTRY() is also in the non-user entry path.
This change is crazy, please ignore it :-S
Christophe
> ---
> arch/powerpc/kernel/entry_32.S | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 0768dfd8a64e..0165edd03b38 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -185,12 +185,6 @@ transfer_to_handler:
> addi r12,r12,-1
> stw r12,4(r11)
> #endif
> -#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> - CURRENT_THREAD_INFO(r9, r1)
> - tophys(r9, r9)
> - ACCOUNT_CPU_USER_ENTRY(r9, r11, r12)
> -#endif
> -
> b 3f
>
> 2: /* if from kernel, check interrupted DOZE/NAP mode and
> @@ -208,9 +202,14 @@ transfer_to_handler:
> bt- 31-TLF_NAPPING,4f
> bt- 31-TLF_SLEEPING,7f
> #endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
> +3:
> +#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> + CURRENT_THREAD_INFO(r9, r1)
> + tophys(r9, r9)
> + ACCOUNT_CPU_USER_ENTRY(r9, r11, r12)
> +#endif
> .globl transfer_to_handler_cont
> transfer_to_handler_cont:
> -3:
> mflr r9
> lwz r11,0(r9) /* virtual address of handler */
> lwz r9,4(r9) /* where to go when done */
>
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Christoph Hellwig @ 2019-01-30 7:44 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jean-Philippe Brucker, Jason Wang, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Christoph Hellwig, Thiago Jung Bauermann, David Gibson
In-Reply-To: <20190129213137-mutt-send-email-mst@kernel.org>
On Tue, Jan 29, 2019 at 09:36:08PM -0500, Michael S. Tsirkin wrote:
> This has been discussed ad nauseum. virtio is all about compatibility.
> Losing a couple of lines of code isn't worth breaking working setups.
> People that want "just use DMA API no tricks" now have the option.
> Setting a flag in a feature bit map is literally a single line
> of code in the hypervisor. So stop pushing for breaking working
> legacy setups and just fix it in the right place.
I agree with the legacy aspect. What I am missing is an extremely
strong wording that says you SHOULD always set this flag for new
hosts, including an explanation why.
^ permalink raw reply
* Does SMP work at all on 40x ?
From: Christophe Leroy @ 2019-01-30 7:16 UTC (permalink / raw)
To: Paul Mackerras, Benjamin Herrenschmidt, Michael Ellerman,
linuxppc-dev@lists.ozlabs.org
In transfer_to_handler() (entry_32.S), we have:
#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
...
#ifdef CONFIG_SMP
CURRENT_THREAD_INFO(r9, r1)
lwz r9,TI_CPU(r9)
slwi r9,r9,3
add r11,r11,r9
#endif
#endif
When running this piece of code, MMU translation is off. But r9 contains
the virtual addr of current_thread_info, so unless I miss something,
this cannot work on the 40x, can it ?
On CONFIG_BOOKE it works because phys addr = virt addr
Christophe
^ permalink raw reply
* Re: [PATCH 05/19] KVM: PPC: Book3S HV: add a new KVM device for the XIVE native exploitation mode
From: Cédric Le Goater @ 2019-01-30 7:01 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190130042919.GA27109@blackberry>
On 1/30/19 5:29 AM, Paul Mackerras wrote:
> On Mon, Jan 28, 2019 at 06:35:34PM +0100, Cédric Le Goater wrote:
>> On 1/22/19 6:05 AM, Paul Mackerras wrote:
>>> On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
>>>> This is the basic framework for the new KVM device supporting the XIVE
>>>> native exploitation mode. The user interface exposes a new capability
>>>> and a new KVM device to be used by QEMU.
>>>
>>> [snip]
>>>> @@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void)
>>>> #ifdef CONFIG_KVM_XIVE
>>>> if (xive_enabled()) {
>>>> kvmppc_xive_init_module();
>>>> + kvmppc_xive_native_init_module();
>>>> kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);
>>>> + kvm_register_device_ops(&kvm_xive_native_ops,
>>>> + KVM_DEV_TYPE_XIVE);
>>>
>>> I think we want tighter conditions on initializing the xive_native
>>> stuff and creating the xive device class. We could have
>>> xive_enabled() returning true in a guest, and this code will get
>>> called both by PR KVM and HV KVM (and HV KVM no longer implies that we
>>> are running bare metal).
>>
>> So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
>> (L1) obviously crashes trying to call OPAL. I have tighten the test with :
>>
>> if (xive_enabled() && !kvmhv_on_pseries()) {
>>
>> for now.
>>
>> As this is a problem today in 5.0.x, I will send a patch for it if you think
>
> How do you mean this is a problem today in 5.0? I just tried 5.0-rc1
> with kernel_irqchip=on in a nested guest and it works just fine. What
> exactly did you test?
L0: Linux 5.0.0-rc3 (+ KVM HV)
L1: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3 (+ KVM HV)
L2: QEMU pseries-4.0 (kernel_irqchip=on) - Linux 5.0.0-rc3
L1 crashes when L2 starts and tries to initialize the KVM IRQ device as
it does an OPAL call and its running under SLOF. See below.
I don't understand how L2 can work with kernel_irqchip=on. Could you
please explain ?
>> it is correct. I don't think we should bother taking care of the PR case
>> on P9. Should we ?
>
> We do need to take care of PR KVM on P9, since it is the only form of
> nested KVM that works inside a host in HPT mode.
ok. That is the test case. There are quite a few combinations now.
Thanks,
C.
[ 49.547056] Oops: Exception in kernel mode, sig: 4 [#1]
[ 49.555101] LE SMP NR_CPUS=2048 NUMA pSeries
[ 49.555132] Modules linked in: xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 xt_tcpudp bridge stp llc ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter vmx_crypto crct10dif_vpmsum crc32c_vpmsum kvm_hv kvm sch_fq_codel ip_tables x_tables autofs4 virtio_net net_failover failover virtio_scsi
[ 49.555335] CPU: 9 PID: 2162 Comm: qemu-system-ppc Kdump: loaded Not tainted 5.0.0-rc3+ #53
[ 49.555378] NIP: c0000000000a7548 LR: c0000000000a4044 CTR: c0000000000a24b0
[ 49.555421] REGS: c0000003ad71f8a0 TRAP: 0700 Not tainted (5.0.0-rc3+)
[ 49.555456] MSR: 8000000000041033 <SF,ME,IR,DR,RI,LE> CR: 44222822 XER: 20040000
[ 49.555501] CFAR: c0000000000a2508 IRQMASK: 0
[ 49.555501] GPR00: 0000000000000087 c0000003ad71fb30 c00000000175f700 000000000000000b
[ 49.555501] GPR04: 0000000000000000 0000000000000000 c0000003f88d4000 000000000000000b
[ 49.555501] GPR08: 00000003fd800000 000000000000000b 0000000000000800 0000000000000031
[ 49.555501] GPR12: 8000000000001002 c000000007ff3280 0000000000000000 0000000000000000
[ 49.555501] GPR16: 00007ffff8d2bd60 0000000000000000 000002c9896d7800 00007ffff8d2b970
[ 49.555501] GPR20: 000002c95c876f90 000002c95c876fa0 000002c95c876f80 000002c95c876f70
[ 49.555501] GPR24: 000002c95cf4f648 ffffffffffffffff c0000003ab3e4058 00000000006000c0
[ 49.555501] GPR28: 000000000000000b c0000003ab3e0000 0000000000000000 c0000003f88d0000
[ 49.555883] NIP [c0000000000a7548] opal_xive_alloc_vp_block+0x50/0x68
[ 49.555919] LR [c0000000000a4044] opal_return+0x0/0x48
[ 49.555947] Call Trace:
[ 49.555964] [c0000003ad71fb30] [c0000000000a250c] xive_native_alloc_vp_block+0x5c/0x1c0 (unreliable)
[ 49.556019] [c0000003ad71fbc0] [c00800000430c0c0] kvmppc_xive_create+0x98/0x168 [kvm]
[ 49.556065] [c0000003ad71fc00] [c0080000042f9fcc] kvm_vm_ioctl+0x474/0xa00 [kvm]
[ 49.556113] [c0000003ad71fd10] [c000000000423a64] do_vfs_ioctl+0xd4/0x8e0
[ 49.556153] [c0000003ad71fdb0] [c000000000424334] ksys_ioctl+0xc4/0x110
[ 49.556190] [c0000003ad71fe00] [c0000000004243a8] sys_ioctl+0x28/0x80
[ 49.556230] [c0000003ad71fe20] [c00000000000b288] system_call+0x5c/0x70
[ 49.556265] Instruction dump:
[ 49.556288] 60000000 7d600026 91610008 39600000 616b8000 f98d0980 7d8c5878 7d810164
[ 49.556332] e9628098 7d6803a6 39600031 7d8c5878 <7d9b4ba6> e96280b0 e98b0008 e84b0000
[ 49.556378] ---[ end trace ac7420a6784de93b ]---
^ permalink raw reply
* Re: [PATCH 05/19] KVM: PPC: Book3S HV: add a new KVM device for the XIVE native exploitation mode
From: Paul Mackerras @ 2019-01-30 4:29 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <d21664e5-1d9c-5a55-5b83-bf5e6247917a@kaod.org>
On Mon, Jan 28, 2019 at 06:35:34PM +0100, Cédric Le Goater wrote:
> On 1/22/19 6:05 AM, Paul Mackerras wrote:
> > On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
> >> This is the basic framework for the new KVM device supporting the XIVE
> >> native exploitation mode. The user interface exposes a new capability
> >> and a new KVM device to be used by QEMU.
> >
> > [snip]
> >> @@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void)
> >> #ifdef CONFIG_KVM_XIVE
> >> if (xive_enabled()) {
> >> kvmppc_xive_init_module();
> >> + kvmppc_xive_native_init_module();
> >> kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);
> >> + kvm_register_device_ops(&kvm_xive_native_ops,
> >> + KVM_DEV_TYPE_XIVE);
> >
> > I think we want tighter conditions on initializing the xive_native
> > stuff and creating the xive device class. We could have
> > xive_enabled() returning true in a guest, and this code will get
> > called both by PR KVM and HV KVM (and HV KVM no longer implies that we
> > are running bare metal).
>
> So yes, I gave nested a try with kernel_irqchip=on and the nested hypervisor
> (L1) obviously crashes trying to call OPAL. I have tighten the test with :
>
> if (xive_enabled() && !kvmhv_on_pseries()) {
>
> for now.
>
> As this is a problem today in 5.0.x, I will send a patch for it if you think
How do you mean this is a problem today in 5.0? I just tried 5.0-rc1
with kernel_irqchip=on in a nested guest and it works just fine. What
exactly did you test?
> it is correct. I don't think we should bother taking care of the PR case
> on P9. Should we ?
We do need to take care of PR KVM on P9, since it is the only form of
nested KVM that works inside a host in HPT mode.
Paul.
^ permalink raw reply
* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Paul Mackerras @ 2019-01-30 5:40 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <f2e9a801-d0a9-bf29-7b3e-29269021ecb4@kaod.org>
On Tue, Jan 29, 2019 at 02:51:05PM +0100, Cédric Le Goater wrote:
> >>> Another general comment is that you seem to have written all this
> >>> code assuming we are using HV KVM in a host running bare-metal.
> >>
> >> Yes. I didn't look at the other configurations. I thought that we could
> >> use the kernel_irqchip=off option to begin with. A couple of checks
> >> are indeed missing.
> >
> > Using kernel_irqchip=off would mean that we would not be able to use
> > the in-kernel XICS emulation, which would have a performance impact.
>
> yes. But it is not supported today. Correct ?
Not correct, it has been working for years, and works in v5.0-rc1 (I
just tested it), at both L0 and L1.
> > We need an explicit capability for XIVE exploitation that can be
> > enabled or disabled on the qemu command line, so that we can enforce a
> > uniform set of capabilities across all the hosts in a migration
> > domain. And it's no good to say we have the capability when all
> > attempts to use it will fail. Therefore the kernel needs to say that
> > it doesn't have the capability in a PR KVM guest or in a nested HV
> > guest.
>
> OK. I will work on adding a KVM_CAP_PPC_NESTED_IRQ_HV capability
> for future use.
That's not what I meant. Why do we need that? I meant that querying
the new KVM_CAP_PPC_IRQ_XIVE capability should return 0 if we are in a
guest. It should only return 1 if we are running bare-metal on a P9.
> >>> However, we could be using PR KVM (either in a bare-metal host or in a
> >>> guest), or we could be doing nested HV KVM where we are using the
> >>> kvm_hv module inside a KVM guest and using special hypercalls for
> >>> controlling our guests.
> >>
> >> Yes.
> >>
> >> It would be good to talk a little about the nested support (offline
> >> may be) to make sure that we are not missing some major interface that
> >> would require a lot of change. If we need to prepare ground, I think
> >> the timing is good.
> >>
> >> The size of the IRQ number space might be a problem. It seems we
> >> would need to increase it considerably to support multiple nested
> >> guests. That said I haven't look much how nested is designed.
> >
> > The current design of nested HV is that the entire non-volatile state
> > of all the nested guests is encapsulated within the state and
> > resources of the L1 hypervisor. That means that if the L1 hypervisor
> > gets migrated, all of its guests go across inside it and there is no
> > extra state that L0 needs to be aware of. That would imply that the
> > VP number space for the nested guests would need to come from within
> > the VP number space for L1; but the amount of VP space we allocate to
> > each guest doesn't seem to be large enough for that to be practical.
>
> If the KVM XIVE device had some information on the max number of CPUs
> provisioned for the guest, we could optimize the VP allocation.
The problem is that we might have 1000 guests running under L0, or we
might have 1 guest running under L0 and 1000 guests running under it,
and we have no way to know which situation to optimize for at the
point where an L1 guest starts. If we had an enormous VP space then
we could just give each L1 guest a large amount of VP space and solve
it that way; but we don't.
Paul.
^ permalink raw reply
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Paul Mackerras @ 2019-01-30 6:20 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <f92f71ee-5c8e-f6b1-dbbf-436bbb0624cd@kaod.org>
On Tue, Jan 29, 2019 at 02:47:55PM +0100, Cédric Le Goater wrote:
> On 1/29/19 3:45 AM, Paul Mackerras wrote:
> > On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
> >> On 1/28/19 7:13 AM, Paul Mackerras wrote:
> >>> Would we end up with too many VMAs if we just used mmap() to
> >>> change the mappings from the software-generated pages to the
> >>> hardware-generated interrupt pages?
> >> The sPAPR IRQ number space is 0x8000 wide now. The first 4K are
> >> dedicated to CPU IPIs and the remaining 4K are for devices. We can
> >
> > Confused. You say the number space has 32768 entries but then imply
> > there are only 8K entries. Do you mean that the API allows for 15-bit
> > IRQ numbers but we are only making using of 8192 of them?
>
> Ouch. My bad. Let's do it again.
>
> The sPAPR IRQ number space is 0x2000 wide :
>
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/spapr_irq.c;h=1da7a32348fced0bd638717022fc37a83fc5e279;hb=HEAD#l396
>
> The first 4K are dedicated to the CPU IPIs and the remaining 4K are for
> devices (which can be extended if needed).
>
> So that's 8192 x 2 ESB pages.
>
> >> extend the last range if needed as these are for MSIs. Dynamic
> >> extensions under KVM should work also.
> >>
> >> This to say that we have with 8K x 2 (trigger+EOI) pages. This is a
> >> lot of mmap(), too much. Also the KVM model needs to be compatible
> >
> > I wasn't suggesting an mmap per IRQ, I meant that the bulk of the
> > space would be covered by a single mmap, overlaid by subsequent mmaps
> > where we need to map real device interrupts.
>
> ok. The same fault handler could be used to populate the VMA with the
> ESB pages.
>
> But it would mean extra work on the QEMU side, which is not needed
> with this patch.
Maybe, but just storing a single vma pointer in our private data is
not a feasible approach. First, you have no control on the lifetime
of the vma and thus this is a use-after-free waiting to happen, and
secondly, there could be multiple vmas that you need to worry about.
Userspace could do multiple mmaps, or could do mprotect or similar on
part of an existing mmap, which would split the vma for the mmap into
multiple vmas. You don't get notified about munmap either as far as I
can tell, so the vma is liable to go away. And it doesn't matter if
QEMU would never do such things; if userspace can provoke a
use-after-free in the kernel using KVM then someone will write a
specially crafted user program to do that.
So we either solve it in userspace, or we have to write and maintain
complex kernel code with deep links into the MM subsystem. I'd much
rather solve it in userspace.
> >> with the QEMU emulated one and it was simpler to have one overall
> >> memory region for the IPI ESBs, one for the END ESBs (if we support
> >> that one day) and one for the TIMA.
> >>
> >>> Are the necessary pages for a PCI
> >>> passthrough device contiguous in both host real space
> >>
> >> They should as they are the PHB4 ESBs.
> >>
> >>> and guest real space ?
> >>
> >> also. That's how we organized the mapping.
> >
> > "How we organized the mapping" is a significant design decision that I
> > haven't seen documented anywhere, and is really needed for
> > understanding what's going on.
>
> OK. I will add comments on that. See below for some description.
>
> There is nothing fancy, it's simply indexed with the interrupt number,
> like for HW, or for the QEMU XIVE emulated model.
>
> >>> If so we'd only need one mmap() for all the device's interrupt
> >>> pages.
> >>
> >> Ah. So we would want to make a special case for the passthrough
> >> device and have a mmap() and a memory region for its ESBs. Hmm.
> >>
> >> Wouldn't that require to hot plug a memory region under the guest ?
> >
> > No; the way that a memory region works is that userspace can do
> > whatever disparate mappings it likes within the region on the user
> > process side, and the corresponding region of guest real address space
> > follows that automatically.
>
> yes. I suppose this should work also for 'ram device' memory mappings.
>
> So when the passthrough device is added to the guest, we would add a
> new 'ram device' memory region for the device interrupt ESB pages
> that would overlap with the initial guest ESB pages.
Not knowing the QEMU internals all that well, I don't at all
understand why a new ram device is necesssary. I would see it as a
single virtual area mapping the ESB pages of guest hwirqs that are in
use, and we manage those mappings with mmap and munmap.
> This is really a different approach.
>
> >> which means that we need to provision an address space/container
> >> region for theses regions. What are the benefits ?
> >>
> >> Is clearing the PTEs and repopulating the VMA unsafe ?
> >
> > Explicitly unmapping parts of the VMA seems like the wrong way to do
> > it. If you have a device mmap where the device wants to change the
> > physical page underlying parts of the mapping, there should be a way
> > for it to do that explicitly (but I don't know off the top of my head
> > what the interface to do that is).
> >
> > However, I still haven't seen a clear and concise explanation of what
> > is being changed, when, and why we need to do that.
>
> Yes. I agree on that. The problem is not very different from what we
> have today with the XICS-over-XIVE glue in KVM. Let me try to explain.
>
>
> The KVM XICS-over-XIVE device and the proposed KVM XIVE native device
> implement an IRQ space for the guest using the generic IPI interrupts
> of the XIVE IC controller. These interrupts are allocated at the OPAL
> level and "mapped" into the guest IRQ number space in the range 0-0x1FFF.
> Interrupt management is performed in the XIVE way: using loads and
> stores on the addresses of the XIVE IPI interrupt ESB pages.
>
> Both KVM devices share the same internal structure caching information
> on the interrupts, among which the xive_irq_data struct containing the
> addresses of the IPI ESB pages and an extra one in case of passthrough.
> The later contains the addresses of the ESB pages of the underlying HW
> controller interrupts, PHB4 in all cases for now.
>
> A guest when running in the XICS legacy interrupt mode lets the KVM
> XICS-over-XIVE device "handle" interrupt management, that is to perform
> the loads and stores on the addresses of the ESB pages of the guest
> interrupts.
>
> However, when running in XIVE native exploitation mode, the KVM XIVE
> native device exposes the interrupt ESB pages to the guest and lets
> the guest perform directly the loads and stores.
>
> The VMA exposing the ESB pages make use of a custom VM fault handler
> which role is to populate the VMA with appropriate pages. When a fault
> occurs, the guest IRQ number is deduced from the offset, and the ESB
> pages of associated XIVE IPI interrupt are inserted in the VMA (using
> the internal structure caching information on the interrupts).
>
> Supporting device passthrough in the guest running in XIVE native
> exploitation mode adds some extra refinements because the ESB pages
> of a different HW controller (PHB4) need to be exposed to the guest
> along with the initial IPI ESB pages of the XIVE IC controller. But
> the overall mechanic is the same.
>
> When the device HW irqs are mapped into or unmapped from the guest
> IRQ number space, the passthru_irq helpers, kvmppc_xive_set_mapped()
> and kvmppc_xive_clr_mapped(), are called to record or clear the
> passthrough interrupt information and to perform the switch.
>
> The approach taken by this patch is to clear the ESB pages of the
> guest IRQ number being mapped and let the VM fault handler repopulate.
> The handler will insert the ESB page corresponding to the HW interrupt
> of the device being passed-through or the initial IPI ESB page if the
> device is being removed.
That's a much better write-up. Thanks.
Paul.
^ permalink raw reply
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Paul Mackerras @ 2019-01-30 5:55 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <2fd9b526-347d-a1d5-925b-bc633231de64@kaod.org>
On Tue, Jan 29, 2019 at 06:44:50PM +0100, Cédric Le Goater wrote:
> On 1/29/19 5:12 AM, Paul Mackerras wrote:
> > On Mon, Jan 28, 2019 at 07:26:00PM +0100, Cédric Le Goater wrote:
> >>
> >> Is clearing the PTEs and repopulating the VMA unsafe ?
> >
> > Actually, now that I come to think of it, there could be any number of
> > VMAs (well, up to almost 64k of them), since once you have a file
> > descriptor you can call mmap on it multiple times.
> >
> > The more I think about it, the more I think that getting userspace to
> > manage the mappings with mmap() and munmap() is the right idea if it
> > can be made to work.
>
> We might be able to mmap() and munmap() regions of ESB pages in the RTAS
> call "ibm,change-msi". I think that's the right spot for it.
I was thinking that the ESB pages should be mmapped for device
interrupts at VM startup or when a device is hot-plugged in, and
munmapped when the device is hot-removed. Maybe the mmap could be
done in conjunction with the KVM_IRQFD call?
What is the reasoning behind doing it in the ibm,change-msi call?
Paul.
^ permalink raw reply
* [RFC PATCH v3 4/4] kvmppc: Handle memory plug/unplug to secure VM
From: Bharata B Rao @ 2019-01-30 6:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: linuxram, kvm-ppc, Bharata B Rao, benh, linux-mm, jglisse,
aneesh.kumar, paulus, sukadev
In-Reply-To: <20190130060726.29958-1-bharata@linux.ibm.com>
Register the new memslot with UV during plug and unregister
the memslot during unplug.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/ucall-api.h | 7 +++++++
arch/powerpc/include/uapi/asm/uapi_uvcall.h | 1 +
arch/powerpc/kvm/book3s_hv.c | 19 +++++++++++++++++++
3 files changed, 27 insertions(+)
diff --git a/arch/powerpc/include/asm/ucall-api.h b/arch/powerpc/include/asm/ucall-api.h
index d266670229cb..cbb8bb38eb8b 100644
--- a/arch/powerpc/include/asm/ucall-api.h
+++ b/arch/powerpc/include/asm/ucall-api.h
@@ -82,5 +82,12 @@ static inline int uv_register_mem_slot(u64 lpid, u64 start_gpa, u64 size,
return plpar_ucall(UV_REGISTER_MEM_SLOT, retbuf, lpid, start_gpa,
size, flags, slotid);
}
+
+static inline int uv_unregister_mem_slot(u64 lpid, u64 slotid)
+{
+ unsigned long retbuf[PLPAR_UCALL_BUFSIZE];
+
+ return plpar_ucall(UV_UNREGISTER_MEM_SLOT, retbuf, lpid, slotid);
+}
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_UCALL_API_H */
diff --git a/arch/powerpc/include/uapi/asm/uapi_uvcall.h b/arch/powerpc/include/uapi/asm/uapi_uvcall.h
index 79a11a6ee436..60e44c7b58c4 100644
--- a/arch/powerpc/include/uapi/asm/uapi_uvcall.h
+++ b/arch/powerpc/include/uapi/asm/uapi_uvcall.h
@@ -13,6 +13,7 @@
#define UV_RESTRICTED_SPR_READ 0xf10C
#define UV_RETURN 0xf11C
#define UV_REGISTER_MEM_SLOT 0xF120
+#define UV_UNREGISTER_MEM_SLOT 0xF124
#define UV_PAGE_IN 0xF128
#define UV_PAGE_OUT 0xF12C
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 1dfb42ac9626..61e36c4516d5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -76,6 +76,7 @@
#include <asm/xive.h>
#include <asm/kvm_host.h>
#include <asm/kvm_book3s_hmm.h>
+#include <asm/ucall-api.h>
#include "book3s.h"
@@ -4433,6 +4434,24 @@ static void kvmppc_core_commit_memory_region_hv(struct kvm *kvm,
if (change == KVM_MR_FLAGS_ONLY && kvm_is_radix(kvm) &&
((new->flags ^ old->flags) & KVM_MEM_LOG_DIRTY_PAGES))
kvmppc_radix_flush_memslot(kvm, old);
+ /*
+ * If UV hasn't yet called H_SVM_INIT_START, don't register memslots.
+ */
+ if (!kvm->arch.secure_guest)
+ return;
+
+ /*
+ * TODO: Handle KVM_MR_MOVE
+ */
+ if (change == KVM_MR_CREATE) {
+ uv_register_mem_slot(kvm->arch.lpid,
+ new->base_gfn << PAGE_SHIFT,
+ new->npages * PAGE_SIZE,
+ 0,
+ new->id);
+ } else if (change == KVM_MR_DELETE) {
+ uv_unregister_mem_slot(kvm->arch.lpid, old->id);
+ }
}
/*
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 3/4] kvmppc: H_SVM_INIT_START and H_SVM_INIT_DONE hcalls
From: Bharata B Rao @ 2019-01-30 6:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: linuxram, kvm-ppc, Bharata B Rao, benh, linux-mm, jglisse,
aneesh.kumar, paulus, sukadev
In-Reply-To: <20190130060726.29958-1-bharata@linux.ibm.com>
H_SVM_INIT_START: Initiate securing a VM
H_SVM_INIT_DONE: Conclude securing a VM
During early guest init, these hcalls will be issued by UV.
As part of these hcalls, [un]register memslots with UV.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 2 ++
arch/powerpc/include/asm/kvm_book3s_hmm.h | 12 ++++++++
arch/powerpc/include/asm/ucall-api.h | 9 ++++++
arch/powerpc/include/uapi/asm/uapi_uvcall.h | 1 +
arch/powerpc/kvm/book3s_hv.c | 7 +++++
arch/powerpc/kvm/book3s_hv_hmm.c | 33 +++++++++++++++++++++
6 files changed, 64 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 05b8536f6653..fa7695928e30 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -343,6 +343,8 @@
/* Platform-specific hcalls used by the Ultravisor */
#define H_SVM_PAGE_IN 0xEF00
#define H_SVM_PAGE_OUT 0xEF04
+#define H_SVM_INIT_START 0xEF08
+#define H_SVM_INIT_DONE 0xEF0C
/* Values for 2nd argument to H_SET_MODE */
#define H_SET_MODE_RESOURCE_SET_CIABR 1
diff --git a/arch/powerpc/include/asm/kvm_book3s_hmm.h b/arch/powerpc/include/asm/kvm_book3s_hmm.h
index e61519c17485..af093f8b86cf 100644
--- a/arch/powerpc/include/asm/kvm_book3s_hmm.h
+++ b/arch/powerpc/include/asm/kvm_book3s_hmm.h
@@ -13,6 +13,8 @@ extern unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
unsigned long gra,
unsigned long flags,
unsigned long page_shift);
+extern unsigned long kvmppc_h_svm_init_start(struct kvm *kvm);
+extern unsigned long kvmppc_h_svm_init_done(struct kvm *kvm);
#else
static inline unsigned long
kvmppc_h_svm_page_in(struct kvm *kvm, unsigned int lpid,
@@ -29,5 +31,15 @@ kvmppc_h_svm_page_out(struct kvm *kvm, unsigned int lpid,
{
return H_UNSUPPORTED;
}
+
+static inline unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
+{
+ return H_UNSUPPORTED;
+}
+
+static inline unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)
+{
+ return H_UNSUPPORTED;
+}
#endif /* CONFIG_PPC_KVM_UV */
#endif /* __POWERPC_KVM_PPC_HMM_H__ */
diff --git a/arch/powerpc/include/asm/ucall-api.h b/arch/powerpc/include/asm/ucall-api.h
index 6c3bddc97b55..d266670229cb 100644
--- a/arch/powerpc/include/asm/ucall-api.h
+++ b/arch/powerpc/include/asm/ucall-api.h
@@ -73,5 +73,14 @@ static inline int uv_page_out(u64 lpid, u64 dst_ra, u64 src_gpa, u64 flags,
return plpar_ucall(UV_PAGE_OUT, retbuf, lpid, dst_ra, src_gpa, flags,
page_shift);
}
+
+static inline int uv_register_mem_slot(u64 lpid, u64 start_gpa, u64 size,
+ u64 flags, u64 slotid)
+{
+ unsigned long retbuf[PLPAR_UCALL_BUFSIZE];
+
+ return plpar_ucall(UV_REGISTER_MEM_SLOT, retbuf, lpid, start_gpa,
+ size, flags, slotid);
+}
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_UCALL_API_H */
diff --git a/arch/powerpc/include/uapi/asm/uapi_uvcall.h b/arch/powerpc/include/uapi/asm/uapi_uvcall.h
index 3a30820663a2..79a11a6ee436 100644
--- a/arch/powerpc/include/uapi/asm/uapi_uvcall.h
+++ b/arch/powerpc/include/uapi/asm/uapi_uvcall.h
@@ -12,6 +12,7 @@
#define UV_RESTRICTED_SPR_WRITE 0xf108
#define UV_RESTRICTED_SPR_READ 0xf10C
#define UV_RETURN 0xf11C
+#define UV_REGISTER_MEM_SLOT 0xF120
#define UV_PAGE_IN 0xF128
#define UV_PAGE_OUT 0xF12C
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index e7edba1ec16a..1dfb42ac9626 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1017,6 +1017,13 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
kvmppc_get_gpr(vcpu, 6),
kvmppc_get_gpr(vcpu, 7));
break;
+ case H_SVM_INIT_START:
+ ret = kvmppc_h_svm_init_start(vcpu->kvm);
+ break;
+ case H_SVM_INIT_DONE:
+ ret = kvmppc_h_svm_init_done(vcpu->kvm);
+ break;
+
default:
return RESUME_HOST;
}
diff --git a/arch/powerpc/kvm/book3s_hv_hmm.c b/arch/powerpc/kvm/book3s_hv_hmm.c
index d8112092a242..b8a980172833 100644
--- a/arch/powerpc/kvm/book3s_hv_hmm.c
+++ b/arch/powerpc/kvm/book3s_hv_hmm.c
@@ -55,6 +55,39 @@ struct kvmppc_hmm_migrate_args {
unsigned long page_shift;
};
+unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
+{
+ struct kvm_memslots *slots;
+ struct kvm_memory_slot *memslot;
+ int ret = H_SUCCESS;
+ int srcu_idx;
+
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+ slots = kvm_memslots(kvm);
+ kvm_for_each_memslot(memslot, slots) {
+ ret = uv_register_mem_slot(kvm->arch.lpid,
+ memslot->base_gfn << PAGE_SHIFT,
+ memslot->npages * PAGE_SIZE,
+ 0, memslot->id);
+ if (ret < 0) {
+ ret = H_PARAMETER; /* TODO: proper retval */
+ goto out;
+ }
+ }
+ kvm->arch.secure_guest = true;
+out:
+ srcu_read_unlock(&kvm->srcu, srcu_idx);
+ return ret;
+}
+
+unsigned long kvmppc_h_svm_init_done(struct kvm *kvm)
+{
+ if (!kvm->arch.secure_guest)
+ return H_UNSUPPORTED;
+
+ return H_SUCCESS;
+}
+
#define KVMPPC_PFN_HMM (0x1ULL << 61)
static inline bool kvmppc_is_hmm_pfn(unsigned long pfn)
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 2/4] kvmppc: Add support for shared pages in HMM driver
From: Bharata B Rao @ 2019-01-30 6:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: linuxram, kvm-ppc, Bharata B Rao, benh, linux-mm, jglisse,
aneesh.kumar, paulus, sukadev
In-Reply-To: <20190130060726.29958-1-bharata@linux.ibm.com>
A secure guest will share some of its pages with hypervisor (Eg. virtio
bounce buffers etc). Support shared pages in HMM driver.
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 3 ++
arch/powerpc/kvm/book3s_hv_hmm.c | 58 +++++++++++++++++++++++++++++--
2 files changed, 58 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 2f6b952deb0f..05b8536f6653 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -337,6 +337,9 @@
#define H_TLB_INVALIDATE 0xF808
#define H_COPY_TOFROM_GUEST 0xF80C
+/* Flags for H_SVM_PAGE_IN */
+#define H_PAGE_IN_SHARED 0x1
+
/* Platform-specific hcalls used by the Ultravisor */
#define H_SVM_PAGE_IN 0xEF00
#define H_SVM_PAGE_OUT 0xEF04
diff --git a/arch/powerpc/kvm/book3s_hv_hmm.c b/arch/powerpc/kvm/book3s_hv_hmm.c
index edc512acebd3..d8112092a242 100644
--- a/arch/powerpc/kvm/book3s_hv_hmm.c
+++ b/arch/powerpc/kvm/book3s_hv_hmm.c
@@ -45,6 +45,7 @@ struct kvmppc_hmm_page_pvt {
unsigned long *rmap;
unsigned int lpid;
unsigned long gpa;
+ bool skip_page_out;
};
struct kvmppc_hmm_migrate_args {
@@ -212,6 +213,45 @@ static const struct migrate_vma_ops kvmppc_hmm_migrate_ops = {
.finalize_and_map = kvmppc_hmm_migrate_finalize_and_map,
};
+/*
+ * Shares the page with HV, thus making it a normal page.
+ *
+ * - If the page is already secure, then provision a new page and share
+ * - If the page is a normal page, share the existing page
+ *
+ * In the former case, uses the HMM fault handler to release the HMM page.
+ */
+static unsigned long
+kvmppc_share_page(struct kvm *kvm, unsigned long *rmap, unsigned long gpa,
+ unsigned long addr, unsigned long page_shift)
+{
+
+ int ret;
+ unsigned int lpid = kvm->arch.lpid;
+ struct page *hmm_page;
+ struct kvmppc_hmm_page_pvt *pvt;
+ unsigned long pfn;
+ int srcu_idx;
+
+ if (kvmppc_is_hmm_pfn(*rmap)) {
+ hmm_page = pfn_to_page(*rmap & ~KVMPPC_PFN_HMM);
+ pvt = (struct kvmppc_hmm_page_pvt *)
+ hmm_devmem_page_get_drvdata(hmm_page);
+ pvt->skip_page_out = true;
+ }
+
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+ pfn = gfn_to_pfn(kvm, gpa >> page_shift);
+ srcu_read_unlock(&kvm->srcu, srcu_idx);
+ if (is_error_noslot_pfn(pfn))
+ return H_PARAMETER;
+
+ ret = uv_page_in(lpid, pfn << page_shift, gpa, 0, page_shift);
+ kvm_release_pfn_clean(pfn);
+
+ return (ret == U_SUCCESS) ? H_SUCCESS : H_PARAMETER;
+}
+
/*
* Move page from normal memory to secure memory.
*/
@@ -242,9 +282,12 @@ kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
end = addr + (1UL << page_shift);
- if (flags)
+ if (flags & ~H_PAGE_IN_SHARED)
return H_P2;
+ if (flags & H_PAGE_IN_SHARED)
+ return kvmppc_share_page(kvm, rmap, gpa, addr, page_shift);
+
args.rmap = rmap;
args.lpid = kvm->arch.lpid;
args.gpa = gpa;
@@ -291,8 +334,17 @@ kvmppc_hmm_fault_migrate_alloc_and_copy(struct vm_area_struct *vma,
hmm_devmem_page_get_drvdata(spage);
pfn = page_to_pfn(dpage);
- ret = uv_page_out(pvt->lpid, pfn << PAGE_SHIFT,
- pvt->gpa, 0, PAGE_SHIFT);
+
+ /*
+ * This same alloc_and_copy() callback is used in two cases:
+ * - When HV touches a secure page, for which we do page-out
+ * - When a secure page is converted to shared page, we touch
+ * the page to essentially discard the HMM page. In this case we
+ * skip page-out.
+ */
+ if (!pvt->skip_page_out)
+ ret = uv_page_out(pvt->lpid, pfn << PAGE_SHIFT,
+ pvt->gpa, 0, PAGE_SHIFT);
if (ret == U_SUCCESS)
*dst_pfn = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
}
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 1/4] kvmppc: HMM backend driver to manage pages of secure guest
From: Bharata B Rao @ 2019-01-30 6:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: linuxram, kvm-ppc, Bharata B Rao, benh, linux-mm, jglisse,
aneesh.kumar, paulus, sukadev
In-Reply-To: <20190130060726.29958-1-bharata@linux.ibm.com>
HMM driver for KVM PPC to manage page transitions of
secure guest via H_SVM_PAGE_IN and H_SVM_PAGE_OUT hcalls.
H_SVM_PAGE_IN: Move the content of a normal page to secure page
H_SVM_PAGE_OUT: Move the content of a secure page to normal page
Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 4 +
arch/powerpc/include/asm/kvm_book3s_hmm.h | 33 ++
arch/powerpc/include/asm/kvm_host.h | 14 +
arch/powerpc/include/asm/ucall-api.h | 19 +
arch/powerpc/include/uapi/asm/uapi_uvcall.h | 3 +
arch/powerpc/kvm/Makefile | 3 +
arch/powerpc/kvm/book3s_hv.c | 22 +
arch/powerpc/kvm/book3s_hv_hmm.c | 474 ++++++++++++++++++++
8 files changed, 572 insertions(+)
create mode 100644 arch/powerpc/include/asm/kvm_book3s_hmm.h
create mode 100644 arch/powerpc/kvm/book3s_hv_hmm.c
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 463c63a9fcf1..2f6b952deb0f 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -337,6 +337,10 @@
#define H_TLB_INVALIDATE 0xF808
#define H_COPY_TOFROM_GUEST 0xF80C
+/* Platform-specific hcalls used by the Ultravisor */
+#define H_SVM_PAGE_IN 0xEF00
+#define H_SVM_PAGE_OUT 0xEF04
+
/* Values for 2nd argument to H_SET_MODE */
#define H_SET_MODE_RESOURCE_SET_CIABR 1
#define H_SET_MODE_RESOURCE_SET_DAWR 2
diff --git a/arch/powerpc/include/asm/kvm_book3s_hmm.h b/arch/powerpc/include/asm/kvm_book3s_hmm.h
new file mode 100644
index 000000000000..e61519c17485
--- /dev/null
+++ b/arch/powerpc/include/asm/kvm_book3s_hmm.h
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+#ifndef __POWERPC_KVM_PPC_HMM_H__
+#define __POWERPC_KVM_PPC_HMM_H__
+
+#ifdef CONFIG_PPC_KVM_UV
+extern unsigned long kvmppc_h_svm_page_in(struct kvm *kvm,
+ unsigned int lpid,
+ unsigned long gra,
+ unsigned long flags,
+ unsigned long page_shift);
+extern unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
+ unsigned int lpid,
+ unsigned long gra,
+ unsigned long flags,
+ unsigned long page_shift);
+#else
+static inline unsigned long
+kvmppc_h_svm_page_in(struct kvm *kvm, unsigned int lpid,
+ unsigned long gra, unsigned long flags,
+ unsigned long page_shift)
+{
+ return H_UNSUPPORTED;
+}
+
+static inline unsigned long
+kvmppc_h_svm_page_out(struct kvm *kvm, unsigned int lpid,
+ unsigned long gra, unsigned long flags,
+ unsigned long page_shift)
+{
+ return H_UNSUPPORTED;
+}
+#endif /* CONFIG_PPC_KVM_UV */
+#endif /* __POWERPC_KVM_PPC_HMM_H__ */
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 162005ae50e2..15ea03852bf1 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -846,4 +846,18 @@ static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
+#ifdef CONFIG_PPC_KVM_UV
+extern int kvmppc_hmm_init(void);
+extern void kvmppc_hmm_free(void);
+extern void kvmppc_hmm_release_pfns(struct kvm_memory_slot *free);
+#else
+static inline int kvmppc_hmm_init(void)
+{
+ return 0;
+}
+
+static inline void kvmppc_hmm_free(void) {}
+static inline void kvmppc_hmm_release_pfns(struct kvm_memory_slot *free) {}
+#endif /* CONFIG_PPC_KVM_UV */
+
#endif /* __POWERPC_KVM_HOST_H__ */
diff --git a/arch/powerpc/include/asm/ucall-api.h b/arch/powerpc/include/asm/ucall-api.h
index 82d8edd1e409..6c3bddc97b55 100644
--- a/arch/powerpc/include/asm/ucall-api.h
+++ b/arch/powerpc/include/asm/ucall-api.h
@@ -9,6 +9,8 @@
#include <linux/of_fdt.h>
#include <linux/libfdt.h>
+#define U_SUCCESS 0
+
extern unsigned int smf_state;
static inline bool smf_enabled(void)
{
@@ -54,5 +56,22 @@ static inline int uv_restricted_spr_read(u64 reg, u64 *val)
return rc;
}
+static inline int uv_page_in(u64 lpid, u64 src_ra, u64 dst_gpa, u64 flags,
+ u64 page_shift)
+{
+ unsigned long retbuf[PLPAR_UCALL_BUFSIZE];
+
+ return plpar_ucall(UV_PAGE_IN, retbuf, lpid, src_ra, dst_gpa, flags,
+ page_shift);
+}
+
+static inline int uv_page_out(u64 lpid, u64 dst_ra, u64 src_gpa, u64 flags,
+ u64 page_shift)
+{
+ unsigned long retbuf[PLPAR_UCALL_BUFSIZE];
+
+ return plpar_ucall(UV_PAGE_OUT, retbuf, lpid, dst_ra, src_gpa, flags,
+ page_shift);
+}
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_UCALL_API_H */
diff --git a/arch/powerpc/include/uapi/asm/uapi_uvcall.h b/arch/powerpc/include/uapi/asm/uapi_uvcall.h
index b657af679ca7..3a30820663a2 100644
--- a/arch/powerpc/include/uapi/asm/uapi_uvcall.h
+++ b/arch/powerpc/include/uapi/asm/uapi_uvcall.h
@@ -12,4 +12,7 @@
#define UV_RESTRICTED_SPR_WRITE 0xf108
#define UV_RESTRICTED_SPR_READ 0xf10C
#define UV_RETURN 0xf11C
+#define UV_PAGE_IN 0xF128
+#define UV_PAGE_OUT 0xF12C
+
#endif /* #ifndef UAPI_UC_H */
diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 64f1135e7732..ed3a9d974059 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -76,6 +76,9 @@ kvm-hv-y += \
book3s_64_mmu_radix.o \
book3s_hv_nested.o
+kvm-hv-$(CONFIG_PPC_KVM_UV) += \
+ book3s_hv_hmm.o
+
kvm-hv-$(CONFIG_PPC_TRANSACTIONAL_MEM) += \
book3s_hv_tm.o
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 5a066fc299e1..e7edba1ec16a 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -74,6 +74,8 @@
#include <asm/opal.h>
#include <asm/xics.h>
#include <asm/xive.h>
+#include <asm/kvm_host.h>
+#include <asm/kvm_book3s_hmm.h>
#include "book3s.h"
@@ -1001,6 +1003,20 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
if (nesting_enabled(vcpu->kvm))
ret = kvmhv_copy_tofrom_guest_nested(vcpu);
break;
+ case H_SVM_PAGE_IN:
+ ret = kvmppc_h_svm_page_in(vcpu->kvm,
+ kvmppc_get_gpr(vcpu, 4),
+ kvmppc_get_gpr(vcpu, 5),
+ kvmppc_get_gpr(vcpu, 6),
+ kvmppc_get_gpr(vcpu, 7));
+ break;
+ case H_SVM_PAGE_OUT:
+ ret = kvmppc_h_svm_page_out(vcpu->kvm,
+ kvmppc_get_gpr(vcpu, 4),
+ kvmppc_get_gpr(vcpu, 5),
+ kvmppc_get_gpr(vcpu, 6),
+ kvmppc_get_gpr(vcpu, 7));
+ break;
default:
return RESUME_HOST;
}
@@ -4354,6 +4370,7 @@ static void kvmppc_core_free_memslot_hv(struct kvm_memory_slot *free,
struct kvm_memory_slot *dont)
{
if (!dont || free->arch.rmap != dont->arch.rmap) {
+ kvmppc_hmm_release_pfns(free);
vfree(free->arch.rmap);
free->arch.rmap = NULL;
}
@@ -5429,11 +5446,16 @@ static int kvmppc_book3s_init_hv(void)
no_mixing_hpt_and_radix = true;
}
+ r = kvmppc_hmm_init();
+ if (r < 0)
+ pr_err("KVM-HV: kvmppc_hmm_init failed %d\n", r);
+
return r;
}
static void kvmppc_book3s_exit_hv(void)
{
+ kvmppc_hmm_free();
kvmppc_free_host_rm_ops();
if (kvmppc_radix_possible())
kvmppc_radix_exit();
diff --git a/arch/powerpc/kvm/book3s_hv_hmm.c b/arch/powerpc/kvm/book3s_hv_hmm.c
new file mode 100644
index 000000000000..edc512acebd3
--- /dev/null
+++ b/arch/powerpc/kvm/book3s_hv_hmm.c
@@ -0,0 +1,474 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HMM driver to manage page migration between normal and secure
+ * memory.
+ *
+ * Based on Jérôme Glisse's HMM dummy driver.
+ *
+ * Copyright 2018 Bharata B Rao, IBM Corp. <bharata@linux.ibm.com>
+ */
+
+/*
+ * A pseries guest can be run as a secure guest on Ultravisor-enabled
+ * POWER platforms. On such platforms, this driver will be used to manage
+ * the movement of guest pages between the normal memory managed by
+ * hypervisor (HV) and secure memory managed by Ultravisor (UV).
+ *
+ * Private ZONE_DEVICE memory equal to the amount of secure memory
+ * available in the platform for running secure guests is created
+ * via a HMM device. The movement of pages between normal and secure
+ * memory is done by ->alloc_and_copy() callback routine of migrate_vma().
+ *
+ * The page-in or page-out requests from UV will come to HV as hcalls and
+ * HV will call back into UV via uvcalls to satisfy these page requests.
+ *
+ * For each page that gets moved into secure memory, a HMM PFN is used
+ * on the HV side and HMM migration PTE corresponding to that PFN would be
+ * populated in the QEMU page tables.
+ */
+
+#include <linux/hmm.h>
+#include <linux/kvm_host.h>
+#include <linux/sched/mm.h>
+#include <asm/ucall-api.h>
+
+struct kvmppc_hmm_device {
+ struct hmm_device *device;
+ struct hmm_devmem *devmem;
+ unsigned long *pfn_bitmap;
+};
+
+static struct kvmppc_hmm_device kvmppc_hmm;
+spinlock_t kvmppc_hmm_lock;
+
+struct kvmppc_hmm_page_pvt {
+ unsigned long *rmap;
+ unsigned int lpid;
+ unsigned long gpa;
+};
+
+struct kvmppc_hmm_migrate_args {
+ unsigned long *rmap;
+ unsigned int lpid;
+ unsigned long gpa;
+ unsigned long page_shift;
+};
+
+#define KVMPPC_PFN_HMM (0x1ULL << 61)
+
+static inline bool kvmppc_is_hmm_pfn(unsigned long pfn)
+{
+ return !!(pfn & KVMPPC_PFN_HMM);
+}
+
+void kvmppc_hmm_release_pfns(struct kvm_memory_slot *free)
+{
+ int i;
+
+ for (i = 0; i < free->npages; i++) {
+ unsigned long *rmap = &free->arch.rmap[i];
+
+ if (kvmppc_is_hmm_pfn(*rmap))
+ put_page(pfn_to_page(*rmap & ~KVMPPC_PFN_HMM));
+ }
+}
+
+/*
+ * Get a free HMM PFN from the pool
+ *
+ * Called when a normal page is moved to secure memory (UV_PAGE_IN). HMM
+ * PFN will be used to keep track of the secure page on HV side.
+ */
+/*
+ * TODO: In this and subsequent functions, we pass around and access
+ * individual elements of kvm_memory_slot->arch.rmap[] without any
+ * protection. Figure out the safe way to access this.
+ */
+static struct page *kvmppc_hmm_get_page(unsigned long *rmap,
+ unsigned long gpa, unsigned int lpid)
+{
+ struct page *dpage = NULL;
+ unsigned long bit, hmm_pfn;
+ unsigned long nr_pfns = kvmppc_hmm.devmem->pfn_last -
+ kvmppc_hmm.devmem->pfn_first;
+ unsigned long flags;
+ struct kvmppc_hmm_page_pvt *pvt;
+
+ if (kvmppc_is_hmm_pfn(*rmap))
+ return NULL;
+
+ spin_lock_irqsave(&kvmppc_hmm_lock, flags);
+ bit = find_first_zero_bit(kvmppc_hmm.pfn_bitmap, nr_pfns);
+ if (bit >= nr_pfns)
+ goto out;
+
+ bitmap_set(kvmppc_hmm.pfn_bitmap, bit, 1);
+ hmm_pfn = bit + kvmppc_hmm.devmem->pfn_first;
+ dpage = pfn_to_page(hmm_pfn);
+
+ if (!trylock_page(dpage))
+ goto out_clear;
+
+ *rmap = hmm_pfn | KVMPPC_PFN_HMM;
+ pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC);
+ if (!pvt)
+ goto out_unlock;
+ pvt->rmap = rmap;
+ pvt->gpa = gpa;
+ pvt->lpid = lpid;
+ hmm_devmem_page_set_drvdata(dpage, (unsigned long)pvt);
+ spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
+
+ get_page(dpage);
+ return dpage;
+
+out_unlock:
+ unlock_page(dpage);
+out_clear:
+ bitmap_clear(kvmppc_hmm.pfn_bitmap,
+ hmm_pfn - kvmppc_hmm.devmem->pfn_first, 1);
+out:
+ spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
+ return NULL;
+}
+
+/*
+ * Release the HMM PFN back to the pool
+ *
+ * Called when secure page becomes a normal page during UV_PAGE_OUT.
+ */
+static void kvmppc_hmm_put_page(struct page *page)
+{
+ unsigned long pfn = page_to_pfn(page);
+ unsigned long flags;
+ struct kvmppc_hmm_page_pvt *pvt;
+
+ spin_lock_irqsave(&kvmppc_hmm_lock, flags);
+ pvt = (struct kvmppc_hmm_page_pvt *)hmm_devmem_page_get_drvdata(page);
+ hmm_devmem_page_set_drvdata(page, 0);
+
+ bitmap_clear(kvmppc_hmm.pfn_bitmap,
+ pfn - kvmppc_hmm.devmem->pfn_first, 1);
+ *(pvt->rmap) = 0;
+ spin_unlock_irqrestore(&kvmppc_hmm_lock, flags);
+ kfree(pvt);
+}
+
+/*
+ * migrate_vma() callback to move page from normal memory to secure memory.
+ *
+ * We don't capture the return value of uv_page_in() here because when
+ * UV asks for a page and then fails to copy it over, we don't care.
+ */
+static void
+kvmppc_hmm_migrate_alloc_and_copy(struct vm_area_struct *vma,
+ const unsigned long *src_pfn,
+ unsigned long *dst_pfn,
+ unsigned long start,
+ unsigned long end,
+ void *private)
+{
+ struct kvmppc_hmm_migrate_args *args = private;
+ struct page *spage = migrate_pfn_to_page(*src_pfn);
+ unsigned long pfn = *src_pfn >> MIGRATE_PFN_SHIFT;
+ struct page *dpage;
+
+ *dst_pfn = 0;
+ if (!(*src_pfn & MIGRATE_PFN_MIGRATE))
+ return;
+
+ dpage = kvmppc_hmm_get_page(args->rmap, args->gpa, args->lpid);
+ if (!dpage)
+ return;
+
+ if (spage)
+ uv_page_in(args->lpid, pfn << args->page_shift,
+ args->gpa, 0, args->page_shift);
+
+ *dst_pfn = migrate_pfn(page_to_pfn(dpage)) |
+ MIGRATE_PFN_DEVICE | MIGRATE_PFN_LOCKED;
+}
+
+/*
+ * This migrate_vma() callback is typically used to updated device
+ * page tables after successful migration. We have nothing to do here.
+ *
+ * Also as we don't care if UV successfully copied over the page in
+ * kvmppc_hmm_migrate_alloc_and_copy(), we don't bother to check
+ * dst_pfn for any errors here.
+ */
+static void
+kvmppc_hmm_migrate_finalize_and_map(struct vm_area_struct *vma,
+ const unsigned long *src_pfn,
+ const unsigned long *dst_pfn,
+ unsigned long start,
+ unsigned long end,
+ void *private)
+{
+}
+
+static const struct migrate_vma_ops kvmppc_hmm_migrate_ops = {
+ .alloc_and_copy = kvmppc_hmm_migrate_alloc_and_copy,
+ .finalize_and_map = kvmppc_hmm_migrate_finalize_and_map,
+};
+
+/*
+ * Move page from normal memory to secure memory.
+ */
+unsigned long
+kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
+ unsigned long flags, unsigned long page_shift)
+{
+ unsigned long addr, end;
+ unsigned long src_pfn, dst_pfn;
+ struct kvmppc_hmm_migrate_args args;
+ struct vm_area_struct *vma;
+ int srcu_idx;
+ unsigned long gfn = gpa >> page_shift;
+ struct kvm_memory_slot *slot;
+ unsigned long *rmap;
+ int ret = H_SUCCESS;
+
+ if (page_shift != PAGE_SHIFT)
+ return H_P3;
+
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+ slot = gfn_to_memslot(kvm, gfn);
+ rmap = &slot->arch.rmap[gfn - slot->base_gfn];
+ addr = gfn_to_hva(kvm, gpa >> page_shift);
+ srcu_read_unlock(&kvm->srcu, srcu_idx);
+ if (kvm_is_error_hva(addr))
+ return H_PARAMETER;
+
+ end = addr + (1UL << page_shift);
+
+ if (flags)
+ return H_P2;
+
+ args.rmap = rmap;
+ args.lpid = kvm->arch.lpid;
+ args.gpa = gpa;
+ args.page_shift = page_shift;
+
+ down_read(&kvm->mm->mmap_sem);
+ vma = find_vma_intersection(kvm->mm, addr, end);
+ if (!vma || vma->vm_start > addr || vma->vm_end < end) {
+ ret = H_PARAMETER;
+ goto out;
+ }
+ ret = migrate_vma(&kvmppc_hmm_migrate_ops, vma, addr, end,
+ &src_pfn, &dst_pfn, &args);
+ if (ret < 0)
+ ret = H_PARAMETER;
+out:
+ up_read(&kvm->mm->mmap_sem);
+ return ret;
+}
+
+static void
+kvmppc_hmm_fault_migrate_alloc_and_copy(struct vm_area_struct *vma,
+ const unsigned long *src_pfn,
+ unsigned long *dst_pfn,
+ unsigned long start,
+ unsigned long end,
+ void *private)
+{
+ struct page *dpage, *spage;
+ struct kvmppc_hmm_page_pvt *pvt;
+ unsigned long pfn;
+ int ret = U_SUCCESS;
+
+ *dst_pfn = MIGRATE_PFN_ERROR;
+ spage = migrate_pfn_to_page(*src_pfn);
+ if (!spage || !(*src_pfn & MIGRATE_PFN_MIGRATE))
+ return;
+ if (!is_zone_device_page(spage))
+ return;
+ dpage = hmm_vma_alloc_locked_page(vma, start);
+ if (!dpage)
+ return;
+ pvt = (struct kvmppc_hmm_page_pvt *)
+ hmm_devmem_page_get_drvdata(spage);
+
+ pfn = page_to_pfn(dpage);
+ ret = uv_page_out(pvt->lpid, pfn << PAGE_SHIFT,
+ pvt->gpa, 0, PAGE_SHIFT);
+ if (ret == U_SUCCESS)
+ *dst_pfn = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
+}
+
+/*
+ * This migrate_vma() callback is typically used to updated device
+ * page tables after successful migration. We have nothing to do here.
+ */
+static void
+kvmppc_hmm_fault_migrate_finalize_and_map(struct vm_area_struct *vma,
+ const unsigned long *src_pfn,
+ const unsigned long *dst_pfn,
+ unsigned long start,
+ unsigned long end,
+ void *private)
+{
+}
+
+static const struct migrate_vma_ops kvmppc_hmm_fault_migrate_ops = {
+ .alloc_and_copy = kvmppc_hmm_fault_migrate_alloc_and_copy,
+ .finalize_and_map = kvmppc_hmm_fault_migrate_finalize_and_map,
+};
+
+/*
+ * Fault handler callback when HV touches any page that has been
+ * moved to secure memory, we ask UV to give back the page by
+ * issuing a UV_PAGE_OUT uvcall.
+ */
+static int kvmppc_hmm_devmem_fault(struct hmm_devmem *devmem,
+ struct vm_area_struct *vma,
+ unsigned long addr,
+ const struct page *page,
+ unsigned int flags,
+ pmd_t *pmdp)
+{
+ unsigned long end = addr + PAGE_SIZE;
+ unsigned long src_pfn, dst_pfn = 0;
+
+ if (migrate_vma(&kvmppc_hmm_fault_migrate_ops, vma, addr, end,
+ &src_pfn, &dst_pfn, NULL))
+ return VM_FAULT_SIGBUS;
+ if (dst_pfn == MIGRATE_PFN_ERROR)
+ return VM_FAULT_SIGBUS;
+ return 0;
+}
+
+static void kvmppc_hmm_devmem_free(struct hmm_devmem *devmem,
+ struct page *page)
+{
+ kvmppc_hmm_put_page(page);
+}
+
+static const struct hmm_devmem_ops kvmppc_hmm_devmem_ops = {
+ .free = kvmppc_hmm_devmem_free,
+ .fault = kvmppc_hmm_devmem_fault,
+};
+
+/*
+ * Move page from secure memory to normal memory.
+ */
+unsigned long
+kvmppc_h_svm_page_out(struct kvm *kvm, unsigned long gpa,
+ unsigned long flags, unsigned long page_shift)
+{
+ unsigned long addr, end;
+ struct vm_area_struct *vma;
+ unsigned long src_pfn, dst_pfn = 0;
+ int srcu_idx;
+ int ret = H_SUCCESS;
+
+ if (page_shift != PAGE_SHIFT)
+ return H_P3;
+
+ if (flags)
+ return H_P2;
+
+ srcu_idx = srcu_read_lock(&kvm->srcu);
+ addr = gfn_to_hva(kvm, gpa >> page_shift);
+ srcu_read_unlock(&kvm->srcu, srcu_idx);
+ if (kvm_is_error_hva(addr))
+ return H_PARAMETER;
+
+ end = addr + (1UL << page_shift);
+
+ down_read(&kvm->mm->mmap_sem);
+ vma = find_vma_intersection(kvm->mm, addr, end);
+ if (!vma || vma->vm_start > addr || vma->vm_end < end) {
+ ret = H_PARAMETER;
+ goto out;
+ }
+ ret = migrate_vma(&kvmppc_hmm_fault_migrate_ops, vma, addr, end,
+ &src_pfn, &dst_pfn, NULL);
+ if (ret < 0)
+ ret = H_PARAMETER;
+out:
+ up_read(&kvm->mm->mmap_sem);
+ return ret;
+}
+
+static u64 kvmppc_get_secmem_size(void)
+{
+ struct device_node *np;
+ int i, len;
+ const __be32 *prop;
+ u64 size = 0;
+
+ np = of_find_node_by_path("/ibm,ultravisor/ibm,uv-firmware");
+ if (!np)
+ goto out;
+
+ prop = of_get_property(np, "secure-memory-ranges", &len);
+ if (!prop)
+ goto out_put;
+
+ for (i = 0; i < len / (sizeof(*prop) * 4); i++)
+ size += of_read_number(prop + (i * 4) + 2, 2);
+
+out_put:
+ of_node_put(np);
+out:
+ return size;
+}
+
+static int kvmppc_hmm_pages_init(void)
+{
+ unsigned long nr_pfns = kvmppc_hmm.devmem->pfn_last -
+ kvmppc_hmm.devmem->pfn_first;
+
+ kvmppc_hmm.pfn_bitmap = kcalloc(BITS_TO_LONGS(nr_pfns),
+ sizeof(unsigned long), GFP_KERNEL);
+ if (!kvmppc_hmm.pfn_bitmap)
+ return -ENOMEM;
+
+ spin_lock_init(&kvmppc_hmm_lock);
+
+ return 0;
+}
+
+int kvmppc_hmm_init(void)
+{
+ int ret = 0;
+ unsigned long size;
+
+ size = kvmppc_get_secmem_size();
+ if (!size) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ kvmppc_hmm.device = hmm_device_new(NULL);
+ if (IS_ERR(kvmppc_hmm.device)) {
+ ret = PTR_ERR(kvmppc_hmm.device);
+ goto out;
+ }
+
+ kvmppc_hmm.devmem = hmm_devmem_add(&kvmppc_hmm_devmem_ops,
+ &kvmppc_hmm.device->device, size);
+ if (IS_ERR(kvmppc_hmm.devmem)) {
+ ret = PTR_ERR(kvmppc_hmm.devmem);
+ goto out_device;
+ }
+ ret = kvmppc_hmm_pages_init();
+ if (ret < 0)
+ goto out_device;
+
+ pr_info("KVMPPC-HMM: Secure Memory size %lx\n", size);
+ return ret;
+
+out_device:
+ hmm_device_put(kvmppc_hmm.device);
+out:
+ return ret;
+}
+
+void kvmppc_hmm_free(void)
+{
+ kfree(kvmppc_hmm.pfn_bitmap);
+ hmm_device_put(kvmppc_hmm.device);
+}
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v3 0/4] kvmppc: HMM backend driver to manage pages of secure guest
From: Bharata B Rao @ 2019-01-30 6:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: linuxram, kvm-ppc, Bharata B Rao, benh, linux-mm, jglisse,
aneesh.kumar, paulus, sukadev
Hi,
A pseries guest can be run as a secure guest on Ultravisor-enabled
POWER platforms. On such platforms, this driver will be used to manage
the movement of guest pages between the normal memory managed by
hypervisor (HV) and secure memory managed by Ultravisor (UV).
Private ZONE_DEVICE memory equal to the amount of secure memory
available in the platform for running secure guests is created
via a HMM device. The movement of pages between normal and secure
memory is done by ->alloc_and_copy() callback routine of migrate_vma().
The page-in or page-out requests from UV will come to HV as hcalls and
HV will call back into UV via uvcalls to satisfy these page requests.
These patches apply and work on the base Ultravisor patches posted by
Ram Pai at https://www.spinics.net/lists/kvm-ppc/msg14981.html
Changes in v3
=============
- Rebased to latest kernel
- Rebased on top of Ram's base Ultravisor patches, so that all the
dependencies are met.
- Get secure memory size from device tree.
- Fix a mm struct leak in page-in and page-out hcalls, thereby
allowing LPID recycling (Thanks to Sukadev Bhattiprolu for pointing
this out)
v2: https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-November/181669.html
Bharata B Rao (4):
kvmppc: HMM backend driver to manage pages of secure guest
kvmppc: Add support for shared pages in HMM driver
kvmppc: H_SVM_INIT_START and H_SVM_INIT_DONE hcalls
kvmppc: Handle memory plug/unplug to secure VM
arch/powerpc/include/asm/hvcall.h | 9 +
arch/powerpc/include/asm/kvm_book3s_hmm.h | 45 ++
arch/powerpc/include/asm/kvm_host.h | 14 +
arch/powerpc/include/asm/ucall-api.h | 35 ++
arch/powerpc/include/uapi/asm/uapi_uvcall.h | 5 +
arch/powerpc/kvm/Makefile | 3 +
arch/powerpc/kvm/book3s_hv.c | 48 ++
arch/powerpc/kvm/book3s_hv_hmm.c | 559 ++++++++++++++++++++
8 files changed, 718 insertions(+)
create mode 100644 arch/powerpc/include/asm/kvm_book3s_hmm.h
create mode 100644 arch/powerpc/kvm/book3s_hv_hmm.c
--
2.17.1
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-01-30 4:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <20190129163415.GA14529@lst.de>
Hi Christoph,
Thanks a lot for the updates. I will test the full branch tomorrow.
Cheers,
Christian
Sent from my iPhone
> On 29. Jan 2019, at 17:34, Christoph Hellwig <hch@lst.de> wrote:
>
>> On Tue, Jan 29, 2019 at 05:14:11PM +0100, Christoph Hellwig wrote:
>>> On Tue, Jan 29, 2019 at 04:03:32PM +0100, Christian Zigotzky wrote:
>>> Hi Christoph,
>>>
>>> I compiled kernels for the X5000 and X1000 from your new branch
>>> 'powerpc-dma.6-debug.2' today. The kernels boot and the P.A. Semi Ethernet
>>> works!
>>
>> Thanks for testing! I'll prepare a new series that adds the other
>> patches on top of this one.
>
> And that was easier than I thought - we just had a few patches left
> in powerpc-dma.6, so I've rebased that branch on top of
> powerpc-dma.6-debug.2:
>
> git://git.infradead.org/users/hch/misc.git powerpc-dma.6
>
> Gitweb:
>
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6
>
> I hope the other patches are simple enough, so just testing the full
> branch checkout should be fine for now.
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Michael S. Tsirkin @ 2019-01-30 3:26 UTC (permalink / raw)
To: Jason Wang
Cc: Jean-Philippe Brucker, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Christoph Hellwig, Thiago Jung Bauermann, David Gibson
In-Reply-To: <94ab8910-594e-a056-c799-5536d8ae2b0e@redhat.com>
On Wed, Jan 30, 2019 at 11:05:42AM +0800, Jason Wang wrote:
>
> On 2019/1/30 上午10:36, Michael S. Tsirkin wrote:
> > On Wed, Jan 30, 2019 at 10:24:01AM +0800, Jason Wang wrote:
> > > On 2019/1/30 上午3:02, Michael S. Tsirkin wrote:
> > > > On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
> > > > > Fixing address of powerpc mailing list.
> > > > >
> > > > > Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > > With Christoph's rework of the DMA API that recently landed, the patch
> > > > > > below is the only change needed in virtio to make it work in a POWER
> > > > > > secure guest under the ultravisor.
> > > > > >
> > > > > > The other change we need (making sure the device's dma_map_ops is NULL
> > > > > > so that the dma-direct/swiotlb code is used) can be made in
> > > > > > powerpc-specific code.
> > > > > >
> > > > > > Of course, I also have patches (soon to be posted as RFC) which hook up
> > > > > > <linux/mem_encrypt.h> to the powerpc secure guest support code.
> > > > > >
> > > > > > What do you think?
> > > > > >
> > > > > > From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
> > > > > > From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> > > > > > Date: Thu, 24 Jan 2019 22:08:02 -0200
> > > > > > Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
> > > > > >
> > > > > > The host can't access the guest memory when it's encrypted, so using
> > > > > > regular memory pages for the ring isn't an option. Go through the DMA API.
> > > > > >
> > > > > > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> > > > Well I think this will come back to bite us (witness xen which is now
> > > > reworking precisely this path - but at least they aren't to blame, xen
> > > > came before ACCESS_PLATFORM).
> > > >
> > > > I also still think the right thing would have been to set
> > > > ACCESS_PLATFORM for all systems where device can't access all memory.
> > > >
> > > > But I also think I don't have the energy to argue about power secure
> > > > guest anymore. So be it for power secure guest since the involved
> > > > engineers disagree with me. Hey I've been wrong in the past ;).
> > > >
> > > > But the name "sev_active" makes me scared because at least AMD guys who
> > > > were doing the sensible thing and setting ACCESS_PLATFORM (unless I'm
> > > > wrong? I reemember distinctly that's so) will likely be affected too.
> > > > We don't want that.
> > > >
> > > > So let's find a way to make sure it's just power secure guest for now
> > > > pls.
> > > >
> > > > I also think we should add a dma_api near features under virtio_device
> > > > such that these hacks can move off data path.
> > >
> > > Anyway the current Xen code is conflict with spec which said:
> > >
> > > "If this feature bit is set to 0, then the device has same access to memory
> > > addresses supplied to it as the driver has. In particular, the device will
> > > always use physical addresses matching addresses used by the driver
> > > (typically meaning physical addresses used by the CPU) and not translated
> > > further, and can access any address supplied to it by the driver. When
> > > clear, this overrides any platform-specific description of whether device
> > > access is limited or translated in any way, e.g. whether an IOMMU may be
> > > present. "
> > >
> > > I wonder how much value that the above description can give us. It's kind of
> > > odd that the behavior of "when the feature is not negotiated" is described
> > > in the spec.
> > Hmm what's odd about it? We need to describe the behaviour is all cases.
>
>
> Well, try to limit the behavior of 'legacy' driver is tricky or even
> impossible. Xen is an exact example for this.
So don't. Xen got grand-fathered in because when that came
along we thought it's a one off thing. Was easier to just
add that as a special case. But really >99% of people
have a hypervisor device with direct guest memory access.
All else is esoterica.
>
> >
> > > Personally I think we can remove the above and then we can
> > > switch to use DMA API unconditionally in guest driver. It may have single
> > > digit regression probably, we can try to overcome it.
> > >
> > > Thanks
> > This has been discussed ad nauseum. virtio is all about compatibility.
> > Losing a couple of lines of code isn't worth breaking working setups.
> > People that want "just use DMA API no tricks" now have the option.
> > Setting a flag in a feature bit map is literally a single line
> > of code in the hypervisor. So stop pushing for breaking working
> > legacy setups and just fix it in the right place.
>
>
> I may miss soemthing, which kind of legacy setup is broken? Do you mean
> using virtio without IOMMU_PLATFORM on platform with IOMMU? We actually
> unbreak this setup.
>
> Thanks
Legacy setups by definition are working setups. The rules are pretty
simple. By default virtio == full guest memory access.
If your access is limited or translated in any way, you use a device
with ACCESS_PLATFORM. When in doubt use ACCESS_PLATFORM.
Xen was there before, and it does not have a flag and it still
wants ACCESS_PLATFORM semantics without setting ACCESS_PLATFORM
sometimes. So we don't want to break existing setups,
and we make an exception in that case.
I don't really see any good reason to make more exceptions.
Nor IMHO should we trust all platform people to know about virtio
and have special kind of DMA API just for virtio.
>
> >
> > > > By the way could you please respond about virtio-iommu and
> > > > why there's no support for ACCESS_PLATFORM on power?
> > > >
> > > > I have Cc'd you on these discussions.
> > > >
> > > >
> > > > Thanks!
> > > >
> > > >
> > > > > > ---
> > > > > > drivers/virtio/virtio_ring.c | 5 ++++-
> > > > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > > index cd7e755484e3..321a27075380 100644
> > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > > > > > * not work without an even larger kludge. Instead, enable
> > > > > > * the DMA API if we're a Xen guest, which at least allows
> > > > > > * all of the sensible Xen configurations to work correctly.
> > > > > > + *
> > > > > > + * Also, if guest memory is encrypted the host can't access
> > > > > > + * it directly. In this case, we'll need to use the DMA API.
> > > > > > */
> > > > > > - if (xen_domain())
> > > > > > + if (xen_domain() || sev_active())
> > > > > > return true;
> > > > > >
> > > > > > return false;
> > > > > --
> > > > > Thiago Jung Bauermann
> > > > > IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Jason Wang @ 2019-01-30 3:05 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jean-Philippe Brucker, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Christoph Hellwig, Thiago Jung Bauermann, David Gibson
In-Reply-To: <20190129213137-mutt-send-email-mst@kernel.org>
On 2019/1/30 上午10:36, Michael S. Tsirkin wrote:
> On Wed, Jan 30, 2019 at 10:24:01AM +0800, Jason Wang wrote:
>> On 2019/1/30 上午3:02, Michael S. Tsirkin wrote:
>>> On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
>>>> Fixing address of powerpc mailing list.
>>>>
>>>> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>>>>
>>>>> Hello,
>>>>>
>>>>> With Christoph's rework of the DMA API that recently landed, the patch
>>>>> below is the only change needed in virtio to make it work in a POWER
>>>>> secure guest under the ultravisor.
>>>>>
>>>>> The other change we need (making sure the device's dma_map_ops is NULL
>>>>> so that the dma-direct/swiotlb code is used) can be made in
>>>>> powerpc-specific code.
>>>>>
>>>>> Of course, I also have patches (soon to be posted as RFC) which hook up
>>>>> <linux/mem_encrypt.h> to the powerpc secure guest support code.
>>>>>
>>>>> What do you think?
>>>>>
>>>>> From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
>>>>> From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>>>> Date: Thu, 24 Jan 2019 22:08:02 -0200
>>>>> Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
>>>>>
>>>>> The host can't access the guest memory when it's encrypted, so using
>>>>> regular memory pages for the ring isn't an option. Go through the DMA API.
>>>>>
>>>>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>> Well I think this will come back to bite us (witness xen which is now
>>> reworking precisely this path - but at least they aren't to blame, xen
>>> came before ACCESS_PLATFORM).
>>>
>>> I also still think the right thing would have been to set
>>> ACCESS_PLATFORM for all systems where device can't access all memory.
>>>
>>> But I also think I don't have the energy to argue about power secure
>>> guest anymore. So be it for power secure guest since the involved
>>> engineers disagree with me. Hey I've been wrong in the past ;).
>>>
>>> But the name "sev_active" makes me scared because at least AMD guys who
>>> were doing the sensible thing and setting ACCESS_PLATFORM (unless I'm
>>> wrong? I reemember distinctly that's so) will likely be affected too.
>>> We don't want that.
>>>
>>> So let's find a way to make sure it's just power secure guest for now
>>> pls.
>>>
>>> I also think we should add a dma_api near features under virtio_device
>>> such that these hacks can move off data path.
>>
>> Anyway the current Xen code is conflict with spec which said:
>>
>> "If this feature bit is set to 0, then the device has same access to memory
>> addresses supplied to it as the driver has. In particular, the device will
>> always use physical addresses matching addresses used by the driver
>> (typically meaning physical addresses used by the CPU) and not translated
>> further, and can access any address supplied to it by the driver. When
>> clear, this overrides any platform-specific description of whether device
>> access is limited or translated in any way, e.g. whether an IOMMU may be
>> present. "
>>
>> I wonder how much value that the above description can give us. It's kind of
>> odd that the behavior of "when the feature is not negotiated" is described
>> in the spec.
> Hmm what's odd about it? We need to describe the behaviour is all cases.
Well, try to limit the behavior of 'legacy' driver is tricky or even
impossible. Xen is an exact example for this.
>
>> Personally I think we can remove the above and then we can
>> switch to use DMA API unconditionally in guest driver. It may have single
>> digit regression probably, we can try to overcome it.
>>
>> Thanks
> This has been discussed ad nauseum. virtio is all about compatibility.
> Losing a couple of lines of code isn't worth breaking working setups.
> People that want "just use DMA API no tricks" now have the option.
> Setting a flag in a feature bit map is literally a single line
> of code in the hypervisor. So stop pushing for breaking working
> legacy setups and just fix it in the right place.
I may miss soemthing, which kind of legacy setup is broken? Do you mean
using virtio without IOMMU_PLATFORM on platform with IOMMU? We actually
unbreak this setup.
Thanks
>
>>> By the way could you please respond about virtio-iommu and
>>> why there's no support for ACCESS_PLATFORM on power?
>>>
>>> I have Cc'd you on these discussions.
>>>
>>>
>>> Thanks!
>>>
>>>
>>>>> ---
>>>>> drivers/virtio/virtio_ring.c | 5 ++++-
>>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>>>> index cd7e755484e3..321a27075380 100644
>>>>> --- a/drivers/virtio/virtio_ring.c
>>>>> +++ b/drivers/virtio/virtio_ring.c
>>>>> @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>>>>> * not work without an even larger kludge. Instead, enable
>>>>> * the DMA API if we're a Xen guest, which at least allows
>>>>> * all of the sensible Xen configurations to work correctly.
>>>>> + *
>>>>> + * Also, if guest memory is encrypted the host can't access
>>>>> + * it directly. In this case, we'll need to use the DMA API.
>>>>> */
>>>>> - if (xen_domain())
>>>>> + if (xen_domain() || sev_active())
>>>>> return true;
>>>>>
>>>>> return false;
>>>> --
>>>> Thiago Jung Bauermann
>>>> IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] powerpc/cell: Remove duplicate header
From: Sabyasachi Gupta @ 2019-01-30 2:40 UTC (permalink / raw)
To: Souptick Joarder
Cc: Arnd Bergmann, Brajeswar Ghosh, linux-kernel, paulus,
linuxppc-dev
In-Reply-To: <CAFqt6zZPXcX+Qk_CMNaBhs9=dTJuzv4nQzoj09xWLbMuT6NvDQ@mail.gmail.com>
On Thu, Jan 17, 2019 at 11:13 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> On Thu, Jan 17, 2019 at 9:49 PM Sabyasachi Gupta
> <sabyasachi.linux@gmail.com> wrote:
> >
> > Remove linux/syscalls.h which is included more than once
> >
> > Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
>
> Acked-by: Souptick Joarder <jrdr.linux@gmail.com>
>
If no further comment, can we get this patch in queue for 5.1 ?
> > ---
> > arch/powerpc/platforms/cell/spu_syscalls.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/cell/spu_syscalls.c b/arch/powerpc/platforms/cell/spu_syscalls.c
> > index 263413a..b95d6af 100644
> > --- a/arch/powerpc/platforms/cell/spu_syscalls.c
> > +++ b/arch/powerpc/platforms/cell/spu_syscalls.c
> > @@ -26,7 +26,6 @@
> > #include <linux/syscalls.h>
> > #include <linux/rcupdate.h>
> > #include <linux/binfmts.h>
> > -#include <linux/syscalls.h>
> >
> > #include <asm/spu.h>
> >
> > --
> > 2.7.4
> >
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: Remove duplicate header
From: Sabyasachi Gupta @ 2019-01-30 2:37 UTC (permalink / raw)
To: Souptick Joarder
Cc: Brajeswar Ghosh, linux-kernel, npiggin, paulus, Oliver, msuchanek,
linuxppc-dev, haren
In-Reply-To: <CAFqt6zbyZnk3vAnRHV0JsXpVF=HxoC-WA9x434R+k36b0vtKzA@mail.gmail.com>
On Fri, Jan 18, 2019 at 3:34 PM Souptick Joarder <jrdr.linux@gmail.com> wrote:
>
> On Thu, Jan 17, 2019 at 9:40 PM Sabyasachi Gupta
> <sabyasachi.linux@gmail.com> wrote:
> >
> > Remove linux/printk.h which is included more than once.
> >
> > Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
>
> Acked-by: Souptick Joarder <jrdr.linux@gmail.com>
>
If no further comment, can we get this patch in queue for 5.1 ?
> > ---
> > arch/powerpc/platforms/powernv/opal.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> > index beed86f..802de0d 100644
> > --- a/arch/powerpc/platforms/powernv/opal.c
> > +++ b/arch/powerpc/platforms/powernv/opal.c
> > @@ -26,7 +26,6 @@
> > #include <linux/memblock.h>
> > #include <linux/kthread.h>
> > #include <linux/freezer.h>
> > -#include <linux/printk.h>
> > #include <linux/kmsg_dump.h>
> > #include <linux/console.h>
> > #include <linux/sched/debug.h>
> > --
> > 2.7.4
> >
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Michael S. Tsirkin @ 2019-01-30 2:36 UTC (permalink / raw)
To: Jason Wang
Cc: Jean-Philippe Brucker, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Christoph Hellwig, Thiago Jung Bauermann, David Gibson
In-Reply-To: <6c68f7f7-1b28-6eba-9b8b-2c32efac9701@redhat.com>
On Wed, Jan 30, 2019 at 10:24:01AM +0800, Jason Wang wrote:
>
> On 2019/1/30 上午3:02, Michael S. Tsirkin wrote:
> > On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
> > > Fixing address of powerpc mailing list.
> > >
> > > Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
> > >
> > > > Hello,
> > > >
> > > > With Christoph's rework of the DMA API that recently landed, the patch
> > > > below is the only change needed in virtio to make it work in a POWER
> > > > secure guest under the ultravisor.
> > > >
> > > > The other change we need (making sure the device's dma_map_ops is NULL
> > > > so that the dma-direct/swiotlb code is used) can be made in
> > > > powerpc-specific code.
> > > >
> > > > Of course, I also have patches (soon to be posted as RFC) which hook up
> > > > <linux/mem_encrypt.h> to the powerpc secure guest support code.
> > > >
> > > > What do you think?
> > > >
> > > > From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
> > > > From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> > > > Date: Thu, 24 Jan 2019 22:08:02 -0200
> > > > Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
> > > >
> > > > The host can't access the guest memory when it's encrypted, so using
> > > > regular memory pages for the ring isn't an option. Go through the DMA API.
> > > >
> > > > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> > Well I think this will come back to bite us (witness xen which is now
> > reworking precisely this path - but at least they aren't to blame, xen
> > came before ACCESS_PLATFORM).
> >
> > I also still think the right thing would have been to set
> > ACCESS_PLATFORM for all systems where device can't access all memory.
> >
> > But I also think I don't have the energy to argue about power secure
> > guest anymore. So be it for power secure guest since the involved
> > engineers disagree with me. Hey I've been wrong in the past ;).
> >
> > But the name "sev_active" makes me scared because at least AMD guys who
> > were doing the sensible thing and setting ACCESS_PLATFORM (unless I'm
> > wrong? I reemember distinctly that's so) will likely be affected too.
> > We don't want that.
> >
> > So let's find a way to make sure it's just power secure guest for now
> > pls.
> >
> > I also think we should add a dma_api near features under virtio_device
> > such that these hacks can move off data path.
>
>
> Anyway the current Xen code is conflict with spec which said:
>
> "If this feature bit is set to 0, then the device has same access to memory
> addresses supplied to it as the driver has. In particular, the device will
> always use physical addresses matching addresses used by the driver
> (typically meaning physical addresses used by the CPU) and not translated
> further, and can access any address supplied to it by the driver. When
> clear, this overrides any platform-specific description of whether device
> access is limited or translated in any way, e.g. whether an IOMMU may be
> present. "
>
> I wonder how much value that the above description can give us. It's kind of
> odd that the behavior of "when the feature is not negotiated" is described
> in the spec.
Hmm what's odd about it? We need to describe the behaviour is all cases.
> Personally I think we can remove the above and then we can
> switch to use DMA API unconditionally in guest driver. It may have single
> digit regression probably, we can try to overcome it.
>
> Thanks
This has been discussed ad nauseum. virtio is all about compatibility.
Losing a couple of lines of code isn't worth breaking working setups.
People that want "just use DMA API no tricks" now have the option.
Setting a flag in a feature bit map is literally a single line
of code in the hypervisor. So stop pushing for breaking working
legacy setups and just fix it in the right place.
>
> >
> > By the way could you please respond about virtio-iommu and
> > why there's no support for ACCESS_PLATFORM on power?
> >
> > I have Cc'd you on these discussions.
> >
> >
> > Thanks!
> >
> >
> > > > ---
> > > > drivers/virtio/virtio_ring.c | 5 ++++-
> > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index cd7e755484e3..321a27075380 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > > > * not work without an even larger kludge. Instead, enable
> > > > * the DMA API if we're a Xen guest, which at least allows
> > > > * all of the sensible Xen configurations to work correctly.
> > > > + *
> > > > + * Also, if guest memory is encrypted the host can't access
> > > > + * it directly. In this case, we'll need to use the DMA API.
> > > > */
> > > > - if (xen_domain())
> > > > + if (xen_domain() || sev_active())
> > > > return true;
> > > >
> > > > return false;
> > >
> > > --
> > > Thiago Jung Bauermann
> > > IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Jason Wang @ 2019-01-30 2:24 UTC (permalink / raw)
To: Michael S. Tsirkin, Thiago Jung Bauermann
Cc: Jean-Philippe Brucker, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Christoph Hellwig, David Gibson
In-Reply-To: <20190129134750-mutt-send-email-mst@kernel.org>
On 2019/1/30 上午3:02, Michael S. Tsirkin wrote:
> On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
>> Fixing address of powerpc mailing list.
>>
>> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>>
>>> Hello,
>>>
>>> With Christoph's rework of the DMA API that recently landed, the patch
>>> below is the only change needed in virtio to make it work in a POWER
>>> secure guest under the ultravisor.
>>>
>>> The other change we need (making sure the device's dma_map_ops is NULL
>>> so that the dma-direct/swiotlb code is used) can be made in
>>> powerpc-specific code.
>>>
>>> Of course, I also have patches (soon to be posted as RFC) which hook up
>>> <linux/mem_encrypt.h> to the powerpc secure guest support code.
>>>
>>> What do you think?
>>>
>>> From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
>>> From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>> Date: Thu, 24 Jan 2019 22:08:02 -0200
>>> Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
>>>
>>> The host can't access the guest memory when it's encrypted, so using
>>> regular memory pages for the ring isn't an option. Go through the DMA API.
>>>
>>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Well I think this will come back to bite us (witness xen which is now
> reworking precisely this path - but at least they aren't to blame, xen
> came before ACCESS_PLATFORM).
>
> I also still think the right thing would have been to set
> ACCESS_PLATFORM for all systems where device can't access all memory.
>
> But I also think I don't have the energy to argue about power secure
> guest anymore. So be it for power secure guest since the involved
> engineers disagree with me. Hey I've been wrong in the past ;).
>
> But the name "sev_active" makes me scared because at least AMD guys who
> were doing the sensible thing and setting ACCESS_PLATFORM (unless I'm
> wrong? I reemember distinctly that's so) will likely be affected too.
> We don't want that.
>
> So let's find a way to make sure it's just power secure guest for now
> pls.
>
> I also think we should add a dma_api near features under virtio_device
> such that these hacks can move off data path.
Anyway the current Xen code is conflict with spec which said:
"If this feature bit is set to 0, then the device has same access to
memory addresses supplied to it as the driver has. In particular, the
device will always use physical addresses matching addresses used by the
driver (typically meaning physical addresses used by the CPU) and not
translated further, and can access any address supplied to it by the
driver. When clear, this overrides any platform-specific description of
whether device access is limited or translated in any way, e.g. whether
an IOMMU may be present. "
I wonder how much value that the above description can give us. It's
kind of odd that the behavior of "when the feature is not negotiated" is
described in the spec. Personally I think we can remove the above and
then we can switch to use DMA API unconditionally in guest driver. It
may have single digit regression probably, we can try to overcome it.
Thanks
>
> By the way could you please respond about virtio-iommu and
> why there's no support for ACCESS_PLATFORM on power?
>
> I have Cc'd you on these discussions.
>
>
> Thanks!
>
>
>>> ---
>>> drivers/virtio/virtio_ring.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index cd7e755484e3..321a27075380 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>>> * not work without an even larger kludge. Instead, enable
>>> * the DMA API if we're a Xen guest, which at least allows
>>> * all of the sensible Xen configurations to work correctly.
>>> + *
>>> + * Also, if guest memory is encrypted the host can't access
>>> + * it directly. In this case, we'll need to use the DMA API.
>>> */
>>> - if (xen_domain())
>>> + if (xen_domain() || sev_active())
>>> return true;
>>>
>>> return false;
>>
>> --
>> Thiago Jung Bauermann
>> IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH 0/4] powerpc/livepatch: reliable stack unwinder fixes
From: Jiri Kosina @ 2019-01-29 23:50 UTC (permalink / raw)
To: Joe Lawrence
Cc: Nicolai Stange, linux-kernel, Torsten Duwe, Josh Poimboeuf,
live-patching, linuxppc-dev
In-Reply-To: <20190122155724.27557-1-joe.lawrence@redhat.com>
On Tue, 22 Jan 2019, Joe Lawrence wrote:
> This patchset fixes a false negative report (ie, unreliable) from the
> ppc64 reliable stack unwinder, discussed here [1] when it may
> inadvertently trip over a stale exception marker left on the stack.
>
> The first two patches fix this bug. Nicolai's change clears the marker
> from the stack when an exception is finished. The next patch modifies
> the unwinder to only look for such on stack elements when the ABI
> guarantees that they will actually be initialized.
>
> The final two patches consist of code cleanups that Nicolai and I
> spotted during the development of the fixes.
>
> Testing included re-running the original test scenario (loading a
> livepatch module on ppc64le) on a 5.0.0-rc2 kernel as well as a RHEL-7
> backport. I ran internal tests on the RHEL-7 backport and no new test
> failures were introduced. I believe that Nicolai has done the same
> with respect to the first patch.
>
> [1] https://lore.kernel.org/lkml/7f468285-b149-37e2-e782-c9e538b997a9@redhat.com/
>
> Joe Lawrence (3):
> powerpc/livepatch: relax reliable stack tracer checks for first-frame
> powerpc/livepatch: small cleanups in save_stack_trace_tsk_reliable()
> powerpc/livepatch: return -ERRNO values in
> save_stack_trace_tsk_reliable()
>
> Nicolai Stange (1):
> powerpc/64s: Clear on-stack exception marker upon exception return
Michael, are you fine with this going through LP tree, or do you plan to
take it through yours?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
From: David Miller @ 2019-01-29 23:33 UTC (permalink / raw)
To: Mathias.Thore; +Cc: David.Gounaris, netdev, leoyang.li, linuxppc-dev
In-Reply-To: <DM6PR10MB3721D7BA109A3823D9DACF1984970@DM6PR10MB3721.namprd10.prod.outlook.com>
From: Mathias Thore <Mathias.Thore@infinera.com>
Date: Tue, 29 Jan 2019 08:07:54 +0000
> Is there a scenario where we are clearing the TX ring but don't want to reset the BQL TX queue?
>
> I think it makes sense to keep it in ucc_geth_free_tx since the
> reason it is needed isn't the timeout per se, but rather the
> clearing of the TX ring. This way, it will be performed no matter
> why the driver ends up calling this function.
I absolutely think the BQL reset should remain in ucc_geth_free_tx().
That way if another callsite for ucc_geth_free_tx() emerges that does
need the BQL queue reset, it won't be "forgotten".
^ permalink raw reply
* Re: [PATCH V7 0/4] mm/kvm/vfio/ppc64: Migrate compound pages out of CMA region
From: Andrew Morton @ 2019-01-29 22:56 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Andrea Arcangeli, Alexey Kardashevskiy, linux-kernel,
Michal Hocko, linux-mm, linuxppc-dev, David Gibson
In-Reply-To: <20190114095438.32470-1-aneesh.kumar@linux.ibm.com>
On Mon, 14 Jan 2019 15:24:32 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> ppc64 use CMA area for the allocation of guest page table (hash page table). We won't
> be able to start guest if we fail to allocate hash page table. We have observed
> hash table allocation failure because we failed to migrate pages out of CMA region
> because they were pinned. This happen when we are using VFIO. VFIO on ppc64 pins
> the entire guest RAM. If the guest RAM pages get allocated out of CMA region, we
> won't be able to migrate those pages. The pages are also pinned for the lifetime of the
> guest.
>
> Currently we support migration of non-compound pages. With THP and with the addition of
> hugetlb migration we can end up allocating compound pages from CMA region. This
> patch series add support for migrating compound pages.
Very little review activity is in evidence. Please identify some
appropriate reviewers and ask them to take a look?
^ permalink raw reply
* Re: [PATCH V7 1/4] mm/cma: Add PF flag to force non cma alloc
From: Andrew Morton @ 2019-01-29 22:52 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Andrea Arcangeli, Alexey Kardashevskiy, linux-kernel,
Michal Hocko, linux-mm, linuxppc-dev, David Gibson
In-Reply-To: <20190114095438.32470-2-aneesh.kumar@linux.ibm.com>
On Mon, 14 Jan 2019 15:24:33 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote:
> This patch adds PF_MEMALLOC_NOCMA which make sure any allocation in that context
> is marked non-movable and hence cannot be satisfied by CMA region.
>
> This is useful with get_user_pages_longterm where we want to take a page pin by
> migrating pages from CMA region. Marking the section PF_MEMALLOC_NOCMA ensures
> that we avoid unnecessary page migration later.
>
> ...
>
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1406,6 +1406,7 @@ extern struct pid *cad_pid;
> #define PF_RANDOMIZE 0x00400000 /* Randomize virtual address space */
> #define PF_SWAPWRITE 0x00800000 /* Allowed to write to swap */
> #define PF_MEMSTALL 0x01000000 /* Stalled due to lack of memory */
> +#define PF_MEMALLOC_NOCMA 0x02000000 /* All allocation request will have _GFP_MOVABLE cleared */
> #define PF_NO_SETAFFINITY 0x04000000 /* Userland is not allowed to meddle with cpus_allowed */
> #define PF_MCE_EARLY 0x08000000 /* Early kill for mce process policy */
> #define PF_MUTEX_TESTER 0x20000000 /* Thread belongs to the rt mutex tester */
This flag has been taken by PF_UMH so I moved it to 0x10000000.
And we have now run out of PF_ flags.
^ permalink raw reply
* Re: [PATCH v1] PCI/AER: use match_string() helper to simplify the code
From: Bjorn Helgaas @ 2019-01-29 22:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Sam Bobroff, linuxppc-dev, Oliver O'Halloran, linux-pci
In-Reply-To: <20190128115728.68240-1-andriy.shevchenko@linux.intel.com>
On Mon, Jan 28, 2019 at 01:57:28PM +0200, Andy Shevchenko wrote:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded implementation.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied to pci/aer for v5.1, thanks!
> ---
> drivers/pci/pcie/aer.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index fed29de783e0..f8fc2114ad39 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -117,7 +117,7 @@ bool pci_aer_available(void)
>
> static int ecrc_policy = ECRC_POLICY_DEFAULT;
>
> -static const char *ecrc_policy_str[] = {
> +static const char * const ecrc_policy_str[] = {
> [ECRC_POLICY_DEFAULT] = "bios",
> [ECRC_POLICY_OFF] = "off",
> [ECRC_POLICY_ON] = "on"
> @@ -203,11 +203,8 @@ void pcie_ecrc_get_policy(char *str)
> {
> int i;
>
> - for (i = 0; i < ARRAY_SIZE(ecrc_policy_str); i++)
> - if (!strncmp(str, ecrc_policy_str[i],
> - strlen(ecrc_policy_str[i])))
> - break;
> - if (i >= ARRAY_SIZE(ecrc_policy_str))
> + i = match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
> + if (i < 0)
> return;
>
> ecrc_policy = i;
> --
> 2.20.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox