* Re: [PATCH v3 02/11] s390x/mm: Fail when an altmap is used for arch_add_memory()
From: David Hildenbrand @ 2019-07-15 10:51 UTC (permalink / raw)
To: Michal Hocko
Cc: Oscar Salvador, linux-s390, linux-ia64, Vasily Gorbik, linux-sh,
Heiko Carstens, linux-kernel, Wei Yang, linux-mm, Mike Rapoport,
Martin Schwidefsky, Igor Mammedov, akpm, linuxppc-dev,
Dan Williams, linux-arm-kernel
In-Reply-To: <20190701124628.GT6376@dhcp22.suse.cz>
On 01.07.19 14:46, Michal Hocko wrote:
> On Mon 01-07-19 09:43:06, Michal Hocko wrote:
>> On Mon 27-05-19 13:11:43, David Hildenbrand wrote:
>>> ZONE_DEVICE is not yet supported, fail if an altmap is passed, so we
>>> don't forget arch_add_memory()/arch_remove_memory() when unlocking
>>> support.
>>
>> Why do we need this? Sure ZONE_DEVICE is not supported for s390 and so
>> might be the case for other arches which support hotplug. I do not see
>> much point in adding warning to each of them.
>
> I would drop this one. If there is a strong reason to have something
> like that it should come with a better explanation and it can be done on
> top.
>
This was requested by Dan and I agree it is the right thing to do. In
the context of paravirtualized devices (e.g., virtio-pmem), it makes
sense to block functionality an arch does not support.
I'll leave the decision to Andrew.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 06/11] mm/memory_hotplug: Allow arch_remove_pages() without CONFIG_MEMORY_HOTREMOVE
From: David Hildenbrand @ 2019-07-15 10:54 UTC (permalink / raw)
To: Michal Hocko
Cc: Oscar Salvador, Rich Felker, linux-ia64, Anshuman Khandual,
linux-sh, Peter Zijlstra, Dave Hansen, Heiko Carstens, Wei Yang,
linux-mm, Arun KS, Paul Mackerras, H. Peter Anvin,
Thomas Gleixner, Qian Cai, linux-s390, Yoshinori Sato,
Rafael J. Wysocki, Mike Rapoport, Ingo Molnar, Fenghua Yu,
Pavel Tatashin, Vasily Gorbik, Rob Herring, mike.travis@hpe.com,
Nicholas Piggin, Alex Deucher, Mark Brown, Borislav Petkov,
Andy Lutomirski, Dan Williams, Chris Wilson, linux-arm-kernel,
Tony Luck, Baoquan He, Masahiro Yamada, Mathieu Malaterre,
Greg Kroah-Hartman, Andrew Banman, linux-kernel, Logan Gunthorpe,
Wei Yang, Martin Schwidefsky, Igor Mammedov, akpm, linuxppc-dev,
David S. Miller, Kirill A. Shutemov
In-Reply-To: <20190701125112.GW6376@dhcp22.suse.cz>
On 01.07.19 14:51, Michal Hocko wrote:
> On Mon 01-07-19 10:01:41, Michal Hocko wrote:
>> On Mon 27-05-19 13:11:47, David Hildenbrand wrote:
>>> We want to improve error handling while adding memory by allowing
>>> to use arch_remove_memory() and __remove_pages() even if
>>> CONFIG_MEMORY_HOTREMOVE is not set to e.g., implement something like:
>>>
>>> arch_add_memory()
>>> rc = do_something();
>>> if (rc) {
>>> arch_remove_memory();
>>> }
>>>
>>> We won't get rid of CONFIG_MEMORY_HOTREMOVE for now, as it will require
>>> quite some dependencies for memory offlining.
>>
>> If we cannot really remove CONFIG_MEMORY_HOTREMOVE altogether then why
>> not simply add an empty placeholder for arch_remove_memory when the
>> config is disabled?
>
> In other words, can we replace this by something as simple as:
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index ae892eef8b82..0329027fe740 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -128,6 +128,20 @@ extern void arch_remove_memory(int nid, u64 start, u64 size,
> struct vmem_altmap *altmap);
> extern void __remove_pages(struct zone *zone, unsigned long start_pfn,
> unsigned long nr_pages, struct vmem_altmap *altmap);
> +#else
> +/*
> + * Allow code using
> + * arch_add_memory();
> + * rc = do_something();
> + * if (rc)
> + * arch_remove_memory();
> + *
> + * without ifdefery.
> + */
> +static inline void arch_remove_memory(int nid, u64 start, u64 size,
> + struct vmem_altmap *altmap)
> +{
> +}
> #endif /* CONFIG_MEMORY_HOTREMOVE */
>
> /*
>
A system configured without CONFIG_MEMORY_HOTREMOVE should not suddenly
behave worse than before when adding of memory fails. What you suggest
result in that.
The goal should be to force architectures to properly implement
arch_remove_memory() right from the start - which is the case for all
architectures after this patch set *except* arm, for which a proper
implementation is on the way.
So I'm leaving it like it is. arch_remove_memory() will be mandatory for
architectures implementing arch_add_memory().
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 09/11] mm/memory_hotplug: Remove memory block devices before arch_remove_memory()
From: David Hildenbrand @ 2019-07-15 10:58 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-ia64, linux-sh, Wei Yang, linux-mm, Arun KS, Ingo Molnar,
linux-s390, Rafael J. Wysocki, Pavel Tatashin,
mike.travis@hpe.com, Mark Brown, Jonathan Cameron, Dan Williams,
Chris Wilson, linux-arm-kernel, Oscar Salvador, Andrew Banman,
Mathieu Malaterre, Greg Kroah-Hartman, linux-kernel, Alex Deucher,
Igor Mammedov, akpm, linuxppc-dev, David S. Miller
In-Reply-To: <20190701084129.GI6376@dhcp22.suse.cz>
On 01.07.19 10:41, Michal Hocko wrote:
> On Mon 27-05-19 13:11:50, David Hildenbrand wrote:
>> Let's factor out removing of memory block devices, which is only
>> necessary for memory added via add_memory() and friends that created
>> memory block devices. Remove the devices before calling
>> arch_remove_memory().
>>
>> This finishes factoring out memory block device handling from
>> arch_add_memory() and arch_remove_memory().
>
> OK, this makes sense again. Just a nit. Calling find_memory_block_by_id
> for each memory block looks a bit suboptimal, especially when we are
> removing consequent physical memblocks. I have to confess that I do not
> know how expensive is the search and I also expect that there won't be
> that many memblocks in the removed range anyway as large setups have
> large memblocks.
>
The devices are not allocated sequentially, so there is no easy way to
look them up.
There is a comment for find_memory_block():
"For now, we have a linear search to go find the appropriate
memory_block corresponding to a particular phys_index. If this gets to
be a real problem, we can always use a radix tree or something here."
So if this becomes a problem, we need a separate data structure to speed
up the lookup. (IOW, this was already the same in the old code)
Thanks!
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 10/11] mm/memory_hotplug: Make unregister_memory_block_under_nodes() never fail
From: David Hildenbrand @ 2019-07-15 11:10 UTC (permalink / raw)
To: Michal Hocko, Oscar Salvador
Cc: linux-s390, linux-ia64, linux-sh, Greg Kroah-Hartman, Mark Brown,
Rafael J. Wysocki, linux-kernel, Wei Yang, linux-mm,
David S. Miller, Jonathan Cameron, Alex Deucher, Igor Mammedov,
akpm, Chris Wilson, linuxppc-dev, Dan Williams, linux-arm-kernel
In-Reply-To: <20190701102756.GO6376@dhcp22.suse.cz>
On 01.07.19 12:27, Michal Hocko wrote:
> On Mon 01-07-19 11:36:44, Oscar Salvador wrote:
>> On Mon, Jul 01, 2019 at 10:51:44AM +0200, Michal Hocko wrote:
>>> Yeah, we do not allow to offline multi zone (node) ranges so the current
>>> code seems to be over engineered.
>>>
>>> Anyway, I am wondering why do we have to strictly check for already
>>> removed nodes links. Is the sysfs code going to complain we we try to
>>> remove again?
>>
>> No, sysfs will silently "fail" if the symlink has already been removed.
>> At least that is what I saw last time I played with it.
>>
>> I guess the question is what if sysfs handling changes in the future
>> and starts dropping warnings when trying to remove a symlink is not there.
>> Maybe that is unlikely to happen?
>
> And maybe we handle it then rather than have a static allocation that
> everybody with hotremove configured has to pay for.
>
So what's the suggestion? Dropping the nodemask_t completely and calling
sysfs_remove_link() on already potentially removed links?
Of course, we can also just use mem_blk->nid and rest assured that it
will never be called for memory blocks belonging to multiple nodes.
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
From: Masahiro Yamada @ 2019-07-15 12:03 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20190715072959.GB20882@gate.crashing.org>
On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
> > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > Yes, that is why I used the environment variable, all binutils work
> > > with that. There was no --target option in GNU ar before 2.22.
I use binutils 2.30
It does not understand --target option.
$ powerpc-linux-ar --version
GNU ar (GNU Binutils) 2.30
Copyright (C) 2018 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.
If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this:
powerpc-linux-ar: -t: No such file or directory
> > Yeah, we're not very good at testing with really old binutils, so I
> > guess we broke that.
> >
> > I'm inclined to merge this, it doesn't seem to break anything, and it
> > fixes using --target on old binutils that don't have it.
>
> But we don't set the target any other way either. I don't think this
> will work with a 32-bit toolchain (default target 32 bit) and a 64-bit
> kernel, or the other way around.
>
> Then again, does that work at *all* nowadays? Do we even consider that
> important, *should* it work?
Let me confirm if I understood this discussion.
[1] KBUILD_ARFLAGS += --target=elf$(BITS)-$(GNUTARGET)
is pointless since it is always overridden by another
KBUILD_ARFLAGS assignment.
[2] If we stop overriding it, it would cause build errors.
So, --target is not only useless, but it is rather harmful.
So, we all agreed with this patch, right?
We are discussing whether or not to revive
GNUTARGET=elf$(BITS)-$(GNUTARGET)
in a *separate* patch, correct?
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH kernel v4 2/4] powerpc/iommu: Allow bypass-only for DMA
From: Christoph Hellwig @ 2019-07-15 12:35 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Sam Bobroff, Alistair Popple, Christoph Hellwig,
Oliver O'Halloran, linuxppc-dev, David Gibson
In-Reply-To: <1e9d2b30-015d-130e-4d83-c222f87afdc1@ozlabs.ru>
On Mon, Jul 15, 2019 at 06:33:00PM +1000, Alexey Kardashevskiy wrote:
>
>
> On 13/07/2019 01:20, Christoph Hellwig wrote:
> > > This skips the 32bit DMA setup check if the bypass is can be selected.
> >
> > That sentence does not parse. I think you need to dop the "can be"
> > based on the actual patch.
>
>
> "the 32bit DMA setup check" is
> "if (!(tbl = get_iommu_table_base(dev)))".
>
> I can rephrase though.
What I mean is to replace
"This skips the 32bit DMA setup check if the bypass is can be selected."
with
"This skips the 32bit DMA setup check if the bypass is selected."
or alternatively:
"This skips the 32bit DMA setup check if the bypass can be selected."
but I think the first version is more accurate.
^ permalink raw reply
* Re: [PATCH kernel v4 0/4 repost] powerpc/ioda2: Yet another attempt to allow DMA masks between 32 and 59
From: Christoph Hellwig @ 2019-07-15 12:39 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Sam Bobroff, Alistair Popple, Christoph Hellwig,
Oliver O'Halloran, linuxppc-dev, David Gibson
In-Reply-To: <44fe9d2e-0793-13f9-e0eb-4ff649e229a5@ozlabs.ru>
On Mon, Jul 15, 2019 at 06:43:12PM +1000, Alexey Kardashevskiy wrote:
> > e.g. if you have a DMA that supports 40-bit DMA addressing we could
> > always treat it as if supports 32-bit addressing,and I thought the
> > powerpc code does that,
>
> powerpc does that and this is what the patchset is changing as people
> complained that 2GB DMA window has bad effects on AMD GPUs (cannot allocate
> enough buffers) and 40/100Gbit devices (lower performance), I do not have
> the details handy.
Make sense. I'm just surprised about the complains from the habalabs
folks, which sounded like a 40something bit DMA mask did not work at all
for them on power9, which did not fit my reading of the code.
>
> > as the DMA API now relies on that.
>
> Relies on what precisely? If a device cannot do full 64bit, then it has to
> be no more than just 32bit?
The fact that if say you iommu only supports mode that return up to
32-bit iova and a driver sets a 48 or 64-bit mask you still return
success instead of letting the driver handle the failure and set a
32-bit mask in the fallback code. As said I think the powerpc code
is fine based on my reading from it.
> > Did I miss
> > something and it explicitly rejected that (in which case I didn't spot
> > the fix in this series), or is this just an optimization to handle these
> > devices more optimally, in which case maybe the changelog could be
> > improved a bit.
>
>
> 4/4 did this essentially:
As long as the above is fine (which I think it is) just make it a little
mor clear that this is a simple optimization, not a bug fix for DMA API
usage.
^ permalink raw reply
* Re: [PATCH 1/2] arch: mark syscall number 435 reserved for clone3
From: Christian Borntraeger @ 2019-07-15 13:56 UTC (permalink / raw)
To: Christian Brauner, linux-kernel
Cc: linux-arch, linux-s390, linux-ia64, linux-parisc, arnd, linux-sh,
Heiko Carstens, linux-mips, linux-m68k, linux-alpha, sparclinux,
linuxppc-dev, Vasily Gorbik
In-Reply-To: <20190714192205.27190-2-christian@brauner.io>
I think Vasily already has a clone3 patch for s390x with 435.
On 14.07.19 21:22, Christian Brauner wrote:
> A while ago Arnd made it possible to give new system calls the same
> syscall number on all architectures (except alpha). To not break this
> nice new feature let's mark 435 for clone3 as reserved on all
> architectures that do not yet implement it.
> Even if an architecture does not plan to implement it this ensures that
> new system calls coming after clone3 will have the same number on all
> architectures.
>
> Signed-off-by: Christian Brauner <christian@brauner.io>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-arch@vger.kernel.org
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linux-m68k@lists.linux-m68k.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Cc: sparclinux@vger.kernel.org
> ---
> arch/alpha/kernel/syscalls/syscall.tbl | 1 +
> arch/ia64/kernel/syscalls/syscall.tbl | 1 +
> arch/m68k/kernel/syscalls/syscall.tbl | 1 +
> arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
> arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
> arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
> arch/parisc/kernel/syscalls/syscall.tbl | 1 +
> arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
> arch/s390/kernel/syscalls/syscall.tbl | 1 +
> arch/sh/kernel/syscalls/syscall.tbl | 1 +
> arch/sparc/kernel/syscalls/syscall.tbl | 1 +
> 11 files changed, 11 insertions(+)
>
> diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> index 1db9bbcfb84e..728fe028c02c 100644
> --- a/arch/alpha/kernel/syscalls/syscall.tbl
> +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> @@ -474,3 +474,4 @@
> 542 common fsmount sys_fsmount
> 543 common fspick sys_fspick
> 544 common pidfd_open sys_pidfd_open
> +# 545 reserved for clone3
> diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl
> index ecc44926737b..36d5faf4c86c 100644
> --- a/arch/ia64/kernel/syscalls/syscall.tbl
> +++ b/arch/ia64/kernel/syscalls/syscall.tbl
> @@ -355,3 +355,4 @@
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
> index 9a3eb2558568..a88a285a0e5f 100644
> --- a/arch/m68k/kernel/syscalls/syscall.tbl
> +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> @@ -434,3 +434,4 @@
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
> index 97035e19ad03..c9c879ec9b6d 100644
> --- a/arch/mips/kernel/syscalls/syscall_n32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
> @@ -373,3 +373,4 @@
> 432 n32 fsmount sys_fsmount
> 433 n32 fspick sys_fspick
> 434 n32 pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
> index d7292722d3b0..bbce9159caa1 100644
> --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> @@ -349,3 +349,4 @@
> 432 n64 fsmount sys_fsmount
> 433 n64 fspick sys_fspick
> 434 n64 pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
> index dba084c92f14..9653591428ec 100644
> --- a/arch/mips/kernel/syscalls/syscall_o32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
> @@ -422,3 +422,4 @@
> 432 o32 fsmount sys_fsmount
> 433 o32 fspick sys_fspick
> 434 o32 pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
> index 5022b9e179c2..c7aadfef5386 100644
> --- a/arch/parisc/kernel/syscalls/syscall.tbl
> +++ b/arch/parisc/kernel/syscalls/syscall.tbl
> @@ -431,3 +431,4 @@
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index f2c3bda2d39f..3331749aab20 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -516,3 +516,4 @@
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> index 6ebacfeaf853..a90d3e945445 100644
> --- a/arch/s390/kernel/syscalls/syscall.tbl
> +++ b/arch/s390/kernel/syscalls/syscall.tbl
> @@ -437,3 +437,4 @@
> 432 common fsmount sys_fsmount sys_fsmount
> 433 common fspick sys_fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
> index 834c9c7d79fa..b5ed26c4c005 100644
> --- a/arch/sh/kernel/syscalls/syscall.tbl
> +++ b/arch/sh/kernel/syscalls/syscall.tbl
> @@ -437,3 +437,4 @@
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
> diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
> index c58e71f21129..8c8cc7537fb2 100644
> --- a/arch/sparc/kernel/syscalls/syscall.tbl
> +++ b/arch/sparc/kernel/syscalls/syscall.tbl
> @@ -480,3 +480,4 @@
> 432 common fsmount sys_fsmount
> 433 common fspick sys_fspick
> 434 common pidfd_open sys_pidfd_open
> +# 435 reserved for clone3
>
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Halil Pasic @ 2019-07-15 14:03 UTC (permalink / raw)
To: Thiago Jung Bauermann, Janosch Frank
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
Borislav Petkov, Lendacky, Thomas, H. Peter Anvin, linux-fsdevel,
Thomas Gleixner, linuxppc-dev, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <87tvbqgboc.fsf@morokweng.localdomain>
On Fri, 12 Jul 2019 18:55:47 -0300
Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
>
> [ Cc'ing Tom Lendacky which I forgot to do earlier. Sorry about that. ]
>
> Hello Halil,
>
> Thanks for the quick review.
>
> Halil Pasic <pasic@linux.ibm.com> writes:
>
> > On Fri, 12 Jul 2019 02:36:31 -0300
> > Thiago Jung Bauermann <bauerman@linux.ibm.com> wrote:
> >
> >> Secure Encrypted Virtualization is an x86-specific feature, so it shouldn't
> >> appear in generic kernel code because it forces non-x86 architectures to
> >> define the sev_active() function, which doesn't make a lot of sense.
> >
> > sev_active() might be just bad (too specific) name for a general
> > concept. s390 code defines it drives the right behavior in
> > kernel/dma/direct.c (which uses it).
>
> I thought about that but couldn't put my finger on a general concept.
> Is it "guest with memory inaccessible to the host"?
>
Well, force_dma_unencrypted() is a much better name thatn sev_active():
s390 has no AMD SEV, that is sure, but for virtio to work we do need to
make our dma accessible to the hypervisor. Yes, your "guest with memory
inaccessible to the host" shows into the right direction IMHO.
Unfortunately I don't have too many cycles to spend on this right now.
> Since your proposed definiton for force_dma_unencrypted() is simply to
> make it equivalent to sev_active(), I thought it was more
> straightforward to make each arch define force_dma_unencrypted()
> directly.
I did not mean to propose equivalence. I intended to say the name
sev_active() is not suitable for a common concept. On the other hand
we do have a common concept -- as common code needs to do or not do
things depending on whether "memory is protected/encrypted" or not. I'm
fine with the name force_dma_unencrypted(), especially because I don't
have a better name.
>
> Also, does sev_active() drive the right behavior for s390 in
> elfcorehdr_read() as well?
>
AFAIU, since s390 does not override it boils down to the same, whether
sev_active() returns true or false. I'm no expert in that area, but I
strongly hope that is the right behavior. @Janosch: can you help me
out with this one?
> >> To solve this problem, add an x86 elfcorehdr_read() function to override
> >> the generic weak implementation. To do that, it's necessary to make
> >> read_from_oldmem() public so that it can be used outside of vmcore.c.
> >>
> >> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> >> ---
> >> arch/x86/kernel/crash_dump_64.c | 5 +++++
> >> fs/proc/vmcore.c | 8 ++++----
> >> include/linux/crash_dump.h | 14 ++++++++++++++
> >> include/linux/mem_encrypt.h | 1 -
> >> 4 files changed, 23 insertions(+), 5 deletions(-)
> >
> > Does not seem to apply to today's or yesterdays master.
>
> It assumes the presence of the two patches I mentioned in the cover
> letter. Only one of them is in master.
>
> I hadn't realized the s390 virtio patches were on their way to upstream.
> I was keeping an eye on the email thread but didn't see they were picked
> up in the s390 pull request. I'll add a new patch to this series making
> the corresponding changes to s390's <asm/mem_encrypt.h> as well.
>
Being on cc for your patch made me realize that things got broken on
s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
to benefit from your cleanups. I think with your cleanups and that patch
of mine both sev_active() and sme_active() can be removed. Feel free to
do so. If not, I can attend to it as well.
Regards,
Halil
^ permalink raw reply
* Re: [PATCH 1/2] arch: mark syscall number 435 reserved for clone3
From: Christian Brauner @ 2019-07-15 14:29 UTC (permalink / raw)
To: Christian Borntraeger
Cc: linux-arch, linux-s390, linux-ia64, linux-parisc, arnd, linux-sh,
Heiko Carstens, linux-kernel, linux-mips, linux-m68k, linux-alpha,
sparclinux, linuxppc-dev, Vasily Gorbik
In-Reply-To: <e14eb2f9-43cb-0b9d-dec4-b7e7dcd62091@de.ibm.com>
On Mon, Jul 15, 2019 at 03:56:04PM +0200, Christian Borntraeger wrote:
> I think Vasily already has a clone3 patch for s390x with 435.
Excellent. I'll leave the # 435 reserved for clone3 on s390x in until
this patch has landed. It shouldn't be a merge conflict and if so it
should be trivial.
Christian
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Christoph Hellwig @ 2019-07-15 14:30 UTC (permalink / raw)
To: Halil Pasic
Cc: linux-s390, Mike Anderson, Janosch Frank, Konrad Rzeszutek Wilk,
Robin Murphy, x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu,
Ingo Molnar, Borislav Petkov, Lendacky, Thomas, H. Peter Anvin,
linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig,
Thiago Jung Bauermann, Marek Szyprowski
In-Reply-To: <20190715160317.7e3dfb33.pasic@linux.ibm.com>
On Mon, Jul 15, 2019 at 04:03:17PM +0200, Halil Pasic wrote:
> > I thought about that but couldn't put my finger on a general concept.
> > Is it "guest with memory inaccessible to the host"?
> >
>
> Well, force_dma_unencrypted() is a much better name thatn sev_active():
> s390 has no AMD SEV, that is sure, but for virtio to work we do need to
> make our dma accessible to the hypervisor. Yes, your "guest with memory
> inaccessible to the host" shows into the right direction IMHO.
> Unfortunately I don't have too many cycles to spend on this right now.
In x86 it means that we need to remove dma encryption using
set_memory_decrypted before using it for DMA purposes. In the SEV
case that seems to be so that the hypervisor can access it, in the SME
case that Tom just fixes it is because there is an encrypted bit set
in the physical address, and if the device doesn't support a large
enough DMA address the direct mapping code has to encrypt the pages
used for the contigous allocation.
> Being on cc for your patch made me realize that things got broken on
> s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
> to benefit from your cleanups. I think with your cleanups and that patch
> of mine both sev_active() and sme_active() can be removed. Feel free to
> do so. If not, I can attend to it as well.
Yes, I think with the dma-mapping fix and this series sme_active and
sev_active should be gone from common code. We should also be able
to remove the exports x86 has for them.
I'll wait a few days and will then feed the dma-mapping fix to Linus,
it might make sense to either rebase Thiagos series on top of the
dma-mapping for-next branch, or wait a few days before reposting.
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Michael S. Tsirkin @ 2019-07-15 14:35 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Mike Anderson, Michael Roth, Jean-Philippe Brucker, Jason Wang,
Alexey Kardashevskiy, Ram Pai, linux-kernel, virtualization,
iommu, linuxppc-dev, Christoph Hellwig, David Gibson
In-Reply-To: <87d0id9nah.fsf@morokweng.localdomain>
On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
>
>
> Michael S. Tsirkin <mst@redhat.com> writes:
>
> > On Thu, Jun 27, 2019 at 10:58:40PM -0300, Thiago Jung Bauermann wrote:
> >>
> >> Michael S. Tsirkin <mst@redhat.com> writes:
> >>
> >> > On Mon, Jun 03, 2019 at 10:13:59PM -0300, Thiago Jung Bauermann wrote:
> >> >>
> >> >>
> >> >> Michael S. Tsirkin <mst@redhat.com> writes:
> >> >>
> >> >> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote:
> >> >> >> I rephrased it in terms of address translation. What do you think of
> >> >> >> this version? The flag name is slightly different too:
> >> >> >>
> >> >> >>
> >> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same
> >> >> >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set,
> >> >> >> with the exception that address translation is guaranteed to be
> >> >> >> unnecessary when accessing memory addresses supplied to the device
> >> >> >> by the driver. Which is to say, 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. This
> >> >> >> flag should be set by the guest if offered, but to allow for
> >> >> >> backward-compatibility device implementations allow for it to be
> >> >> >> left unset by the guest. It is an error to set both this flag and
> >> >> >> VIRTIO_F_ACCESS_PLATFORM.
> >> >> >
> >> >> >
> >> >> >
> >> >> >
> >> >> > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged
> >> >> > drivers. This is why devices fail when it's not negotiated.
> >> >>
> >> >> Just to clarify, what do you mean by unprivileged drivers? Is it drivers
> >> >> implemented in guest userspace such as with VFIO? Or unprivileged in
> >> >> some other sense such as needing to use bounce buffers for some reason?
> >> >
> >> > I had drivers in guest userspace in mind.
> >>
> >> Great. Thanks for clarifying.
> >>
> >> I don't think this flag would work for guest userspace drivers. Should I
> >> add a note about that in the flag definition?
> >
> > I think you need to clarify access protection rules. Is it only
> > translation that is bypassed or is any platform-specific
> > protection mechanism bypassed too?
>
> It is only translation. In a secure guest, if the device tries to access
> a memory address that wasn't provided by the driver then the
> architecture will deny that access. If the device accesses addresses
> provided to it by the driver, then there's no protection mechanism or
> translation to get in the way.
>
> >> >> > This confuses me.
> >> >> > If driver is unpriveledged then what happens with this flag?
> >> >> > It can supply any address it wants. Will that corrupt kernel
> >> >> > memory?
> >> >>
> >> >> Not needing address translation doesn't necessarily mean that there's no
> >> >> IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's
> >> >> always an IOMMU present. And we also support VFIO drivers. The VFIO API
> >> >> for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls
> >> >> to program the IOMMU.
> >> >>
> >> >> For our use case, we don't need address translation because we set up an
> >> >> identity mapping in the IOMMU so that the device can use guest physical
> >> >> addresses.
> >
> > OK so I think I am beginning to see it in a different light. Right now the specific
> > platform creates an identity mapping. That in turn means DMA API can be
> > fast - it does not need to do anything. What you are looking for is a
> > way to tell host it's an identity mapping - just as an optimization.
> >
> > Is that right?
>
> Almost. Theoretically it is just an optimization. But in practice the
> pseries boot firmware (SLOF) doesn't support IOMMU_PLATFORM so it's not
> possible to boot a guest from a device with that flag set.
>
> > So this is what I would call this option:
> >
> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
> >
> > and the explanation should state that all device
> > addresses are translated by the platform to identical
> > addresses.
> >
> > In fact this option then becomes more, not less restrictive
> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
> > by guest to only create identity mappings,
> > and only before driver_ok is set.
> > This option then would always be negotiated together with
> > VIRTIO_F_ACCESS_PLATFORM.
> >
> > Host then must verify that
> > 1. full 1:1 mappings are created before driver_ok
> > or can we make sure this happens before features_ok?
> > that would be ideal as we could require that features_ok fails
> > 2. mappings are not modified between driver_ok and reset
> > i guess attempts to change them will fail -
> > possibly by causing a guest crash
> > or some other kind of platform-specific error
>
> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring
> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
> SLOF as I mentioned above, another is that we would be requiring all
> guests running on the machine (secure guests or not, since we would use
> the same configuration for all guests) to support it. But
> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about
> it and wouldn't be able to use the device.
OK and your target is to enable use with kernel drivers within
guests, right?
My question is, we are defining a new flag here, I guess old guests
then do not set it. How does it help old guests? Or maybe it's
not designed to ...
> > So far so good, but now a question:
> >
> > how are we handling guest address width limitations?
> > Is VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS subject to
> > guest address width limitations?
> > I am guessing we can make them so ...
> > This needs to be documented.
>
> I'm not sure. I will get back to you on this.
>
> >> > And can it access any guest physical address?
> >>
> >> Sorry, I was mistaken. We do support VFIO in guests but not for virtio
> >> devices, only for regular PCI devices. In which case they will use
> >> address translation.
> >
> > Not sure how this answers the question.
>
> Because I had said that we had VFIO virtio drivers, you asked:
>
> > >> > This confuses me.
> > >> > If driver is unpriveledged then what happens with this flag?
> > >> > It can supply any address it wants. Will that corrupt kernel
> > >> > memory?
>
> Since we can't actually have VFIO virtio drivers, there's nothing to
> corrupt the kernel memory.
>
> >> >> If the guest kernel is concerned that an unprivileged driver could
> >> >> jeopardize its integrity it should not negotiate this feature flag.
> >> >
> >> > Unfortunately flag negotiation is done through config space
> >> > and so can be overwritten by the driver.
> >>
> >> Ok, so the guest kernel has to forbid VFIO access on devices where this
> >> flag is advertised.
> >
> > That's possible in theory but in practice we did not yet teach VFIO not
> > to attach to legacy devices without VIRTIO_F_ACCESS_PLATFORM. So all
> > security relies on host denying driver_ok without
> > VIRTIO_F_ACCESS_PLATFORM. New options that bypass guest security are
> > thus tricky as they can create security holes for existing guests.
> > I'm open to ideas about how to do this in a safe way,
>
> If the new flag isn't coupled with ACCESS_PLATFORM then the existing
> mechanism of the host denying driver_ok when ACCESS_PLATFORM isn't set
> will be enough.
>
> >> >> Perhaps there should be a note about this in the flag definition? This
> >> >> concern is platform-dependant though. I don't believe it's an issue in
> >> >> pseries.
> >> >
> >> > Again ACCESS_PLATFORM has a pretty open definition. It does actually
> >> > say it's all up to the platform.
> >> >
> >> > Specifically how will VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION be
> >> > implemented portably? virtio has no portable way to know
> >> > whether DMA API bypasses translation.
> >>
> >> The fact that VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION is set
> >> communicates that knowledge to virtio. There is a shared understanding
> >> between the guest and the host about what this flag being set means.
> >
> > Right but I wonder how are you going to *actually* implement it on Linux?
> > Are you adding a new set of DMA APIs that do everything except
> > translation?
>
> Actually it's the opposite. There's nothing to do in the guest besides
> setting up SWIOTLB and sharing its buffer with the host.
>
> Normally on pseries, devices use the dma_iommu_ops defined in
> arch/powerpc/kernel/dma-iommu.c. I have a patch which changes the
> device's dma_ops to NULL so that the default DMA path will be used:
>
> https://lore.kernel.org/linuxppc-dev/20190713060023.8479-12-bauerman@linux.ibm.com/
>
> Then another patch forces use of SWIOTLB and defines the
> set_memory_{encrypted,decrypted} functions so that SWIOTLB can make its
> buffer be shared with the host:
>
> https://lore.kernel.org/linuxppc-dev/20190713060023.8479-13-bauerman@linux.ibm.com/
>
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
From: janani @ 2019-07-15 15:23 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, x86, Thomas Gleixner, Linuxppc-dev,
Konrad Rzeszutek Wilk, linuxppc-dev, Mike Anderson, Ram Pai,
linux-kernel, Christoph Hellwig, Halil Pasic, iommu, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Lendacky,
Robin Murphy, Alexey Dobriyan, Marek Szyprowski
In-Reply-To: <20190713044554.28719-2-bauerman@linux.ibm.com>
On 2019-07-12 23:45, Thiago Jung Bauermann wrote:
> powerpc is also going to use this feature, so put it in a generic
> location.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/Kconfig | 3 +++
> arch/s390/Kconfig | 3 ---
> arch/x86/Kconfig | 4 +---
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index c47b328eada0..4ef3499d4480 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
> the chance of application behavior change because of timing
> differences. The counts are reported via debugfs.
>
> +config ARCH_HAS_MEM_ENCRYPT
> + bool
> +
> source "kernel/gcov/Kconfig"
>
> source "scripts/gcc-plugins/Kconfig"
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 5d8570ed6cab..f820e631bf89 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -1,7 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> -config ARCH_HAS_MEM_ENCRYPT
> - def_bool y
> -
Since you are removing the "def_bool y" when ARCH_HAS_MEM_ENCRYPT is
moved to arch/Kconfig, does the s390/Kconfig need "select
ARCH_HAS_MEM_ENCRYPT" added like you do for x86/Kconfig?
- Janani
> config MMU
> def_bool y
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c9f331bb538b..5d3295f2df94 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -68,6 +68,7 @@ config X86
> select ARCH_HAS_FORTIFY_SOURCE
> select ARCH_HAS_GCOV_PROFILE_ALL
> select ARCH_HAS_KCOV if X86_64
> + select ARCH_HAS_MEM_ENCRYPT
> select ARCH_HAS_MEMBARRIER_SYNC_CORE
> select ARCH_HAS_PMEM_API if X86_64
> select ARCH_HAS_PTE_SPECIAL
> @@ -1520,9 +1521,6 @@ config X86_CPA_STATISTICS
> helps to determine the effectiveness of preserving large and huge
> page mappings when mapping protections are changed.
>
> -config ARCH_HAS_MEM_ENCRYPT
> - def_bool y
> -
> config AMD_MEM_ENCRYPT
> bool "AMD Secure Memory Encryption (SME) support"
> depends on X86_64 && CPU_SUP_AMD
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Lendacky, Thomas @ 2019-07-15 15:44 UTC (permalink / raw)
To: Christoph Hellwig, Halil Pasic
Cc: linux-s390@vger.kernel.org, Mike Anderson, Janosch Frank,
Konrad Rzeszutek Wilk, Robin Murphy, x86@kernel.org, Ram Pai,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
Ingo Molnar, Borislav Petkov, H. Peter Anvin,
linux-fsdevel@vger.kernel.org, Thomas Gleixner,
linuxppc-dev@lists.ozlabs.org, Alexey Dobriyan,
Thiago Jung Bauermann, Marek Szyprowski
In-Reply-To: <20190715143039.GA6892@lst.de>
On 7/15/19 9:30 AM, Christoph Hellwig wrote:
> On Mon, Jul 15, 2019 at 04:03:17PM +0200, Halil Pasic wrote:
>>> I thought about that but couldn't put my finger on a general concept.
>>> Is it "guest with memory inaccessible to the host"?
>>>
>>
>> Well, force_dma_unencrypted() is a much better name thatn sev_active():
>> s390 has no AMD SEV, that is sure, but for virtio to work we do need to
>> make our dma accessible to the hypervisor. Yes, your "guest with memory
>> inaccessible to the host" shows into the right direction IMHO.
>> Unfortunately I don't have too many cycles to spend on this right now.
>
> In x86 it means that we need to remove dma encryption using
> set_memory_decrypted before using it for DMA purposes. In the SEV
> case that seems to be so that the hypervisor can access it, in the SME
> case that Tom just fixes it is because there is an encrypted bit set
> in the physical address, and if the device doesn't support a large
> enough DMA address the direct mapping code has to encrypt the pages
> used for the contigous allocation.
Just a correction/clarification...
For SME, when a device doesn't support a large enough DMA address to
accommodate the encryption bit as part of the DMA address, the direct
mapping code has to provide un-encrypted pages. For un-encrypted pages,
the DMA address now does not include the encryption bit, making it
acceptable to the device. Since the device is now using a DMA address
without the encryption bit, the physical address in the CPU page table
must match (the call to set_memory_decrypted) so that both the device and
the CPU interact in the same way with the memory.
Thanks,
Tom
>
>> Being on cc for your patch made me realize that things got broken on
>> s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
>> to benefit from your cleanups. I think with your cleanups and that patch
>> of mine both sev_active() and sme_active() can be removed. Feel free to
>> do so. If not, I can attend to it as well.
>
> Yes, I think with the dma-mapping fix and this series sme_active and
> sev_active should be gone from common code. We should also be able
> to remove the exports x86 has for them.
>
> I'll wait a few days and will then feed the dma-mapping fix to Linus,
> it might make sense to either rebase Thiagos series on top of the
> dma-mapping for-next branch, or wait a few days before reposting.
>
^ permalink raw reply
* Re: [PATCH 2/4] powerpc/32: activate ARCH_HAS_PMEM_API and ARCH_HAS_UACCESS_FLUSHCACHE
From: Christophe Leroy @ 2019-07-15 17:03 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, Oliver O'Halloran, Paul Mackerras, linuxppc-dev
In-Reply-To: <87y30z94hp.fsf@concordia.ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> PPC32 also have flush_dcache_range() so it can also support
>> ARCH_HAS_PMEM_API and ARCH_HAS_UACCESS_FLUSHCACHE without changes.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> arch/powerpc/Kconfig | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index d7996cfaceca..cf6e30f637be 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -127,13 +127,13 @@ config PPC
>> select ARCH_HAS_KCOV
>> select ARCH_HAS_MMIOWB if PPC64
>> select ARCH_HAS_PHYS_TO_DMA
>> - select ARCH_HAS_PMEM_API if PPC64
>> + select ARCH_HAS_PMEM_API
>> select ARCH_HAS_PTE_SPECIAL
>> select ARCH_HAS_MEMBARRIER_CALLBACKS
>> select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC64
>> select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) &&
>> !RELOCATABLE && !HIBERNATION)
>> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>> - select ARCH_HAS_UACCESS_FLUSHCACHE if PPC64
>> + select ARCH_HAS_UACCESS_FLUSHCACHE
>> select ARCH_HAS_UBSAN_SANITIZE_ALL
>> select ARCH_HAS_ZONE_DEVICE if PPC_BOOK3S_64
>> select ARCH_HAVE_NMI_SAFE_CMPXCHG
>
> This didn't build for me, probably due to something that's changed in
> the long period between you posting it and me applying it?
>
> corenet32_smp_defconfig:
>
> powerpc64-unknown-linux-gnu-ld: lib/iov_iter.o: in function
> `_copy_from_iter_flushcache':
> powerpc64-unknown-linux-gnu-ld:
> /scratch/michael/build/maint/build~/../lib/iov_iter.c:825: undefined
> reference to `memcpy_page_flushcache'
> powerpc64-unknown-linux-gnu-ld:
> /scratch/michael/build/maint/build~/../lib/iov_iter.c:825: undefined
> reference to `memcpy_flushcache'
> powerpc64-unknown-linux-gnu-ld:
> /scratch/michael/build/maint/build~/../lib/iov_iter.c:825: undefined
> reference to `__copy_from_user_flushcache'
> powerpc64-unknown-linux-gnu-ld:
> /scratch/michael/build/maint/build~/../lib/iov_iter.c:825: undefined
> reference to `memcpy_flushcache'
>
Looks like arch/powerpc/lib/Makefile only builds pmem.o for ppc64
Moving it from obj64-y to obj-y should do it.
I'll update the patch when I'm back in two weeks unless you can fix it before.
Christophe
^ permalink raw reply
* Re: [PATCH v2 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Hoan Tran OS @ 2019-07-15 17:55 UTC (permalink / raw)
To: Michal Hocko, Will Deacon
Cc: Heiko Carstens, open list:MEMORY MANAGEMENT, Paul Mackerras,
H . Peter Anvin, sparclinux@vger.kernel.org, Alexander Duyck,
linux-s390@vger.kernel.org, x86@kernel.org, willy@infradead.org,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
linux-arm-kernel@lists.infradead.org, Catalin Marinas,
Open Source Submission, Pavel Tatashin, Vasily Gorbik,
Will Deacon, Borislav Petkov, Thomas Gleixner, Vlastimil Babka,
Oscar Salvador, linux-kernel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David S . Miller
In-Reply-To: <20190712150007.GU29483@dhcp22.suse.cz>
Hi,
On 7/12/19 10:00 PM, Michal Hocko wrote:
> On Fri 12-07-19 15:37:30, Will Deacon wrote:
>> Hi all,
>>
>> On Fri, Jul 12, 2019 at 02:12:23PM +0200, Michal Hocko wrote:
>>> On Fri 12-07-19 10:56:47, Hoan Tran OS wrote:
>>> [...]
>>>> It would be good if we can enable it by-default. Otherwise, let arch
>>>> enables it by them-self. Do you have any suggestions?
>>>
>>> I can hardly make any suggestions when it is not really clear _why_ you
>>> want to remove this config option in the first place. Please explain
>>> what motivated you to make this change.
>>
>> Sorry, I think this confusion might actually be my fault and Hoan has just
>> been implementing my vague suggestion here:
>>
>> https://lore.kernel.org/linux-arm-kernel/20190625101245.s4vxfosoop52gl4e@willie-the-truck/
>>
>> If the preference of the mm folks is to leave CONFIG_NODES_SPAN_OTHER_NODES
>> as it is, then we can define it for arm64. I just find it a bit weird that
>> the majority of NUMA-capable architectures have to add a symbol in the arch
>> Kconfig file, for what appears to be a performance optimisation applicable
>> only to ia64, mips and sh.
>>
>> At the very least we could make the thing selectable.
>
> Hmm, I thought this was selectable. But I am obviously wrong here.
> Looking more closely, it seems that this is indeed only about
> __early_pfn_to_nid and as such not something that should add a config
> symbol. This should have been called out in the changelog though.
Yes, do you have any other comments about my patch?
>
> Also while at it, does HAVE_MEMBLOCK_NODE_MAP fall into a similar
> bucket? Do we have any NUMA architecture that doesn't enable it?
>
As I checked with arch Kconfig files, there are 2 architectures, riscv
and microblaze, do not support NUMA but enable this config.
And 1 architecture, alpha, supports NUMA but does not enable this config.
Thanks and Regards
Hoan
> Thanks!
>
^ permalink raw reply
* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
From: Segher Boessenkool @ 2019-07-15 18:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Stephen Rothwell, Linux Kernel Mailing List, Nicholas Piggin,
Paul Mackerras, linuxppc-dev
In-Reply-To: <CAK7LNATGEK9wxz87J3sTNOYPdtAFXaegQU9EctEBGULQL-ZC4w@mail.gmail.com>
On Mon, Jul 15, 2019 at 09:03:46PM +0900, Masahiro Yamada wrote:
> On Mon, Jul 15, 2019 at 4:30 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Mon, Jul 15, 2019 at 05:05:34PM +1000, Michael Ellerman wrote:
> > > Segher Boessenkool <segher@kernel.crashing.org> writes:
> > > > Yes, that is why I used the environment variable, all binutils work
> > > > with that. There was no --target option in GNU ar before 2.22.
>
> I use binutils 2.30
> It does not understand --target option.
>
> $ powerpc-linux-ar --version
> GNU ar (GNU Binutils) 2.30
> Copyright (C) 2018 Free Software Foundation, Inc.
> This program is free software; you may redistribute it under the terms of
> the GNU General Public License version 3 or (at your option) any later version.
> This program has absolutely no warranty.
>
> If I give --target=elf$(BITS)-$(GNUTARGET) option, I see this:
> powerpc-linux-ar: -t: No such file or directory
You need to provide a valid command line, like
$ powerpc-linux-ar tv smth.a --target=elf32-powerpc
ar is a bit weird.
Segher
^ permalink raw reply
* [PATCH v4] powerpc/setup_64: fix -Wempty-body warnings
From: Qian Cai @ 2019-07-15 18:32 UTC (permalink / raw)
To: mpe; +Cc: linux-kernel, paulus, tyreld, joe, Qian Cai, linuxppc-dev
At the beginning of setup_64.c, it has,
#ifdef DEBUG
#define DBG(fmt...) udbg_printf(fmt)
#else
#define DBG(fmt...)
#endif
where DBG() could be compiled away, and generate warnings,
arch/powerpc/kernel/setup_64.c: In function 'initialize_cache_info':
arch/powerpc/kernel/setup_64.c:579:49: warning: suggest braces around
empty body in an 'if' statement [-Wempty-body]
DBG("Argh, can't find dcache properties !\n");
^
arch/powerpc/kernel/setup_64.c:582:49: warning: suggest braces around
empty body in an 'if' statement [-Wempty-body]
DBG("Argh, can't find icache properties !\n");
Fix it by using the suggestions from Michael:
"Neither of those sites should use DBG(), that's not really early boot
code, they should just use pr_warn().
And the other uses of DBG() in initialize_cache_info() should just be
removed.
In smp_release_cpus() the entry/exit DBG's should just be removed, and
the spinning_secondaries line should just be pr_debug().
That would just leave the two calls in early_setup(). If we taught
udbg_printf() to return early when udbg_putc is NULL, then we could just
call udbg_printf() unconditionally and get rid of the DBG macro
entirely."
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Qian Cai <cai@lca.pw>
---
v4: Use the suggestions from Michael and __func__ per checkpatch.
v3: Use no_printk() macro, and make sure that format and argument are always
verified by the compiler using a more generic form ##__VA_ARGS__ per Joe.
v2: Fix it by using a NOP while loop per Tyrel.
arch/powerpc/kernel/setup_64.c | 26 ++++++--------------------
arch/powerpc/kernel/udbg.c | 14 ++++++++------
2 files changed, 14 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 44b4c432a273..d2af4c228970 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -68,12 +68,6 @@
#include "setup.h"
-#ifdef DEBUG
-#define DBG(fmt...) udbg_printf(fmt)
-#else
-#define DBG(fmt...)
-#endif
-
int spinning_secondaries;
u64 ppc64_pft_size;
@@ -305,7 +299,7 @@ void __init early_setup(unsigned long dt_ptr)
/* Enable early debugging if any specified (see udbg.h) */
udbg_early_init();
- DBG(" -> early_setup(), dt_ptr: 0x%lx\n", dt_ptr);
+ udbg_printf(" -> %s(), dt_ptr: 0x%lx\n", __func__, dt_ptr);
/*
* Do early initialization using the flattened device
@@ -362,11 +356,11 @@ void __init early_setup(unsigned long dt_ptr)
*/
this_cpu_enable_ftrace();
- DBG(" <- early_setup()\n");
+ udbg_printf(" <- %s()\n", __func__);
#ifdef CONFIG_PPC_EARLY_DEBUG_BOOTX
/*
- * This needs to be done *last* (after the above DBG() even)
+ * This needs to be done *last* (after the above udbg_printf() even)
*
* Right after we return from this function, we turn on the MMU
* which means the real-mode access trick that btext does will
@@ -436,8 +430,6 @@ void smp_release_cpus(void)
if (!use_spinloop())
return;
- DBG(" -> smp_release_cpus()\n");
-
/* All secondary cpus are spinning on a common spinloop, release them
* all now so they can start to spin on their individual paca
* spinloops. For non SMP kernels, the secondary cpus never get out
@@ -456,9 +448,7 @@ void smp_release_cpus(void)
break;
udelay(1);
}
- DBG("spinning_secondaries = %d\n", spinning_secondaries);
-
- DBG(" <- smp_release_cpus()\n");
+ pr_debug("spinning_secondaries = %d\n", spinning_secondaries);
}
#endif /* CONFIG_SMP || CONFIG_KEXEC_CORE */
@@ -551,8 +541,6 @@ void __init initialize_cache_info(void)
struct device_node *cpu = NULL, *l2, *l3 = NULL;
u32 pvr;
- DBG(" -> initialize_cache_info()\n");
-
/*
* All shipping POWER8 machines have a firmware bug that
* puts incorrect information in the device-tree. This will
@@ -576,10 +564,10 @@ void __init initialize_cache_info(void)
*/
if (cpu) {
if (!parse_cache_info(cpu, false, &ppc64_caches.l1d))
- DBG("Argh, can't find dcache properties !\n");
+ pr_warn("Argh, can't find dcache properties !\n");
if (!parse_cache_info(cpu, true, &ppc64_caches.l1i))
- DBG("Argh, can't find icache properties !\n");
+ pr_warn("Argh, can't find icache properties !\n");
/*
* Try to find the L2 and L3 if any. Assume they are
@@ -604,8 +592,6 @@ void __init initialize_cache_info(void)
cur_cpu_spec->dcache_bsize = dcache_bsize;
cur_cpu_spec->icache_bsize = icache_bsize;
-
- DBG(" <- initialize_cache_info()\n");
}
/*
diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
index a384e7c8b01c..01595e8cafe7 100644
--- a/arch/powerpc/kernel/udbg.c
+++ b/arch/powerpc/kernel/udbg.c
@@ -120,13 +120,15 @@ int udbg_write(const char *s, int n)
#define UDBG_BUFSIZE 256
void udbg_printf(const char *fmt, ...)
{
- char buf[UDBG_BUFSIZE];
- va_list args;
+ if (udbg_putc) {
+ char buf[UDBG_BUFSIZE];
+ va_list args;
- va_start(args, fmt);
- vsnprintf(buf, UDBG_BUFSIZE, fmt, args);
- udbg_puts(buf);
- va_end(args);
+ va_start(args, fmt);
+ vsnprintf(buf, UDBG_BUFSIZE, fmt, args);
+ udbg_puts(buf);
+ va_end(args);
+ }
}
void __init udbg_progress(char *s, unsigned short hex)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/imc: Dont create debugfs files for cpu-less nodes
From: Qian Cai @ 2019-07-15 18:41 UTC (permalink / raw)
To: Michael Ellerman, Madhavan Srinivasan; +Cc: linuxppc-dev
In-Reply-To: <87d0ihgojp.fsf@concordia.ellerman.id.au>
On Thu, 2019-07-11 at 14:53 +1000, Michael Ellerman wrote:
> Hi Maddy,
>
> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> > diff --git a/arch/powerpc/platforms/powernv/opal-imc.c
> > b/arch/powerpc/platforms/powernv/opal-imc.c
> > index 186109bdd41b..e04b20625cb9 100644
> > --- a/arch/powerpc/platforms/powernv/opal-imc.c
> > +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> > @@ -69,20 +69,20 @@ static void export_imc_mode_and_cmd(struct device_node
> > *node,
> > if (of_property_read_u32(node, "cb_offset", &cb_offset))
> > cb_offset = IMC_CNTL_BLK_OFFSET;
> >
> > - for_each_node(nid) {
> > - loc = (u64)(pmu_ptr->mem_info[chip].vbase) + cb_offset;
> > + while (ptr->vbase != NULL) {
>
> This means you'll bail out as soon as you find a node with no vbase, but
> it's possible we could have a CPU-less node intermingled with other
> nodes.
>
> So I think you want to keep the for loop, but continue if you see a NULL
> vbase?
Not sure if this will also takes care of some of those messages during the boot
on today's linux-next even without this patch.
[ 18.077780][ T1] debugfs: Directory 'imc' with parent 'powerpc' already
present!
where it has the following layout,
# numactl -H
available: 6 nodes (0,8,252-255)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52
53 54 55 56 57 58 59 60 61 62 63
node 0 size: 130197 MB
node 0 free: 127453 MB
node 8 cpus: 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85
86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108
109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127
node 8 size: 130778 MB
node 8 free: 128119 MB
node 252 cpus:
node 252 size: 0 MB
node 252 free: 0 MB
node 253 cpus:
node 253 size: 0 MB
node 253 free: 0 MB
node 254 cpus:
node 254 size: 0 MB
node 254 free: 0 MB
node 255 cpus:
node 255 size: 0 MB
node 255 free: 0 MB
node distances:
node 0 8 252 253 254 255
0: 10 40 80 80 80 80
8: 40 10 80 80 80 80
252: 80 80 10 80 80 80
253: 80 80 80 10 80 80
254: 80 80 80 80 10 80
255: 80 80 80 80 80 10
>
>
> > + loc = (u64)(ptr->vbase) + cb_offset;
> > imc_mode_addr = (u64 *)(loc + IMC_CNTL_BLK_MODE_OFFSET);
> > - sprintf(mode, "imc_mode_%d", nid);
> > + sprintf(mode, "imc_mode_%d", (u32)(ptr->id));
> > if (!imc_debugfs_create_x64(mode, 0600, imc_debugfs_parent,
> > imc_mode_addr))
> > goto err;
> >
> > imc_cmd_addr = (u64 *)(loc + IMC_CNTL_BLK_CMD_OFFSET);
> > - sprintf(cmd, "imc_cmd_%d", nid);
> > + sprintf(cmd, "imc_cmd_%d", (u32)(ptr->id));
> > if (!imc_debugfs_create_x64(cmd, 0600, imc_debugfs_parent,
> > imc_cmd_addr))
> > goto err;
> > - chip++;
> > + ptr++;
> > }
> > return;
>
> cheers
^ permalink raw reply
* Re: [PATCH v2 3/5] x86: Kconfig: Remove CONFIG_NODES_SPAN_OTHER_NODES
From: Thomas Gleixner @ 2019-07-15 18:43 UTC (permalink / raw)
To: Hoan Tran OS
Cc: Michal Hocko, Catalin Marinas, Heiko Carstens,
open list:MEMORY MANAGEMENT, Paul Mackerras, H . Peter Anvin,
sparclinux@vger.kernel.org, Alexander Duyck,
linux-s390@vger.kernel.org, x86@kernel.org, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Vlastimil Babka,
Open Source Submission, Pavel Tatashin, Vasily Gorbik,
Will Deacon, Borislav Petkov,
linux-arm-kernel@lists.infradead.org, Oscar Salvador,
linux-kernel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David S . Miller
In-Reply-To: <1562887528-5896-4-git-send-email-Hoan@os.amperecomputing.com>
On Thu, 11 Jul 2019, Hoan Tran OS wrote:
> Remove CONFIG_NODES_SPAN_OTHER_NODES as it's enabled
> by default with NUMA.
As I told you before this does not mention that the option is now enabled
even for x86(32bit) configurations which did not enable it before and does
not longer depend on X86_64_ACPI_NUMA.
And there is still no rationale why this makes sense.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 1/3] x86, s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
From: Thiago Jung Bauermann @ 2019-07-15 20:00 UTC (permalink / raw)
To: janani
Cc: linux-s390, x86, Thomas Gleixner, Linuxppc-dev,
Konrad Rzeszutek Wilk, linuxppc-dev, Mike Anderson, Ram Pai,
linux-kernel, Christoph Hellwig, Halil Pasic, iommu, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Lendacky,
Robin Murphy, Alexey Dobriyan, Marek Szyprowski
In-Reply-To: <3dc137a99c73b1b6582fc854844a417e@linux.vnet.ibm.com>
Hello Janani,
Thanks for reviewing the patch.
janani <janani@linux.ibm.com> writes:
> On 2019-07-12 23:45, Thiago Jung Bauermann wrote:
>> powerpc is also going to use this feature, so put it in a generic location.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> arch/Kconfig | 3 +++
>> arch/s390/Kconfig | 3 ---
>> arch/x86/Kconfig | 4 +---
>> 3 files changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index c47b328eada0..4ef3499d4480 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
>> the chance of application behavior change because of timing
>> differences. The counts are reported via debugfs.
>>
>> +config ARCH_HAS_MEM_ENCRYPT
>> + bool
>> +
>> source "kernel/gcov/Kconfig"
>>
>> source "scripts/gcc-plugins/Kconfig"
>> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>> index 5d8570ed6cab..f820e631bf89 100644
>> --- a/arch/s390/Kconfig
>> +++ b/arch/s390/Kconfig
>> @@ -1,7 +1,4 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -config ARCH_HAS_MEM_ENCRYPT
>> - def_bool y
>> -
>
> Since you are removing the "def_bool y" when ARCH_HAS_MEM_ENCRYPT is moved to
> arch/Kconfig, does the s390/Kconfig need "select ARCH_HAS_MEM_ENCRYPT" added
> like you do for x86/Kconfig?
Indeed, I missed that. Thanks for spotting it!
>
> - Janani
>
>> config MMU
>> def_bool y
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c9f331bb538b..5d3295f2df94 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -68,6 +68,7 @@ config X86
>> select ARCH_HAS_FORTIFY_SOURCE
>> select ARCH_HAS_GCOV_PROFILE_ALL
>> select ARCH_HAS_KCOV if X86_64
>> + select ARCH_HAS_MEM_ENCRYPT
>> select ARCH_HAS_MEMBARRIER_SYNC_CORE
>> select ARCH_HAS_PMEM_API if X86_64
>> select ARCH_HAS_PTE_SPECIAL
>> @@ -1520,9 +1521,6 @@ config X86_CPA_STATISTICS
>> helps to determine the effectiveness of preserving large and huge
>> page mappings when mapping protections are changed.
>>
>> -config ARCH_HAS_MEM_ENCRYPT
>> - def_bool y
>> -
>> config AMD_MEM_ENCRYPT
>> bool "AMD Secure Memory Encryption (SME) support"
>> depends on X86_64 && CPU_SUP_AMD
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Thiago Jung Bauermann @ 2019-07-15 20:14 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, Mike Anderson, Janosch Frank, Konrad Rzeszutek Wilk,
Robin Murphy, x86, Ram Pai, linux-kernel, Halil Pasic, iommu,
Ingo Molnar, Borislav Petkov, Lendacky, Thomas, H. Peter Anvin,
linux-fsdevel, Thomas Gleixner, linuxppc-dev, Alexey Dobriyan,
Marek Szyprowski
In-Reply-To: <20190715143039.GA6892@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> On Mon, Jul 15, 2019 at 04:03:17PM +0200, Halil Pasic wrote:
>> > I thought about that but couldn't put my finger on a general concept.
>> > Is it "guest with memory inaccessible to the host"?
>> >
>>
>> Well, force_dma_unencrypted() is a much better name thatn sev_active():
>> s390 has no AMD SEV, that is sure, but for virtio to work we do need to
>> make our dma accessible to the hypervisor. Yes, your "guest with memory
>> inaccessible to the host" shows into the right direction IMHO.
>> Unfortunately I don't have too many cycles to spend on this right now.
>
> In x86 it means that we need to remove dma encryption using
> set_memory_decrypted before using it for DMA purposes. In the SEV
> case that seems to be so that the hypervisor can access it, in the SME
> case that Tom just fixes it is because there is an encrypted bit set
> in the physical address, and if the device doesn't support a large
> enough DMA address the direct mapping code has to encrypt the pages
> used for the contigous allocation.
>
>> Being on cc for your patch made me realize that things got broken on
>> s390. Thanks! I've sent out a patch that fixes protvirt, but we are going
>> to benefit from your cleanups. I think with your cleanups and that patch
>> of mine both sev_active() and sme_active() can be removed. Feel free to
>> do so. If not, I can attend to it as well.
>
> Yes, I think with the dma-mapping fix and this series sme_active and
> sev_active should be gone from common code. We should also be able
> to remove the exports x86 has for them.
>
> I'll wait a few days and will then feed the dma-mapping fix to Linus,
> it might make sense to either rebase Thiagos series on top of the
> dma-mapping for-next branch, or wait a few days before reposting.
I'll rebase on top of dma-mapping/for-next and do the break up of patch
2 that you mentioned as well.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Thiago Jung Bauermann @ 2019-07-15 20:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mike Anderson, Michael Roth, Jean-Philippe Brucker, Jason Wang,
Alexey Kardashevskiy, Ram Pai, linux-kernel, virtualization,
iommu, linuxppc-dev, Christoph Hellwig, David Gibson
In-Reply-To: <20190715103411-mutt-send-email-mst@kernel.org>
Michael S. Tsirkin <mst@redhat.com> writes:
> On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
>>
>>
>> Michael S. Tsirkin <mst@redhat.com> writes:
>>
>> > So this is what I would call this option:
>> >
>> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
>> >
>> > and the explanation should state that all device
>> > addresses are translated by the platform to identical
>> > addresses.
>> >
>> > In fact this option then becomes more, not less restrictive
>> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
>> > by guest to only create identity mappings,
>> > and only before driver_ok is set.
>> > This option then would always be negotiated together with
>> > VIRTIO_F_ACCESS_PLATFORM.
>> >
>> > Host then must verify that
>> > 1. full 1:1 mappings are created before driver_ok
>> > or can we make sure this happens before features_ok?
>> > that would be ideal as we could require that features_ok fails
>> > 2. mappings are not modified between driver_ok and reset
>> > i guess attempts to change them will fail -
>> > possibly by causing a guest crash
>> > or some other kind of platform-specific error
>>
>> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring
>> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
>> SLOF as I mentioned above, another is that we would be requiring all
>> guests running on the machine (secure guests or not, since we would use
>> the same configuration for all guests) to support it. But
>> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
>> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about
>> it and wouldn't be able to use the device.
>
> OK and your target is to enable use with kernel drivers within
> guests, right?
Right.
> My question is, we are defining a new flag here, I guess old guests
> then do not set it. How does it help old guests? Or maybe it's
> not designed to ...
Indeed. The idea is that QEMU can offer the flag, old guests can reject
it (or even new guests can reject it, if they decide not to convert into
secure VMs) and the feature negotiation will succeed with the flag
unset.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Michael S. Tsirkin @ 2019-07-15 20:36 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Mike Anderson, Michael Roth, Jean-Philippe Brucker, Jason Wang,
Alexey Kardashevskiy, Ram Pai, linux-kernel, virtualization,
iommu, linuxppc-dev, Christoph Hellwig, David Gibson
In-Reply-To: <874l3nnist.fsf@morokweng.localdomain>
On Mon, Jul 15, 2019 at 05:29:06PM -0300, Thiago Jung Bauermann wrote:
>
> Michael S. Tsirkin <mst@redhat.com> writes:
>
> > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
> >>
> >>
> >> Michael S. Tsirkin <mst@redhat.com> writes:
> >>
> >> > So this is what I would call this option:
> >> >
> >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
> >> >
> >> > and the explanation should state that all device
> >> > addresses are translated by the platform to identical
> >> > addresses.
> >> >
> >> > In fact this option then becomes more, not less restrictive
> >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
> >> > by guest to only create identity mappings,
> >> > and only before driver_ok is set.
> >> > This option then would always be negotiated together with
> >> > VIRTIO_F_ACCESS_PLATFORM.
> >> >
> >> > Host then must verify that
> >> > 1. full 1:1 mappings are created before driver_ok
> >> > or can we make sure this happens before features_ok?
> >> > that would be ideal as we could require that features_ok fails
> >> > 2. mappings are not modified between driver_ok and reset
> >> > i guess attempts to change them will fail -
> >> > possibly by causing a guest crash
> >> > or some other kind of platform-specific error
> >>
> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring
> >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
> >> SLOF as I mentioned above, another is that we would be requiring all
> >> guests running on the machine (secure guests or not, since we would use
> >> the same configuration for all guests) to support it. But
> >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
> >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about
> >> it and wouldn't be able to use the device.
> >
> > OK and your target is to enable use with kernel drivers within
> > guests, right?
>
> Right.
>
> > My question is, we are defining a new flag here, I guess old guests
> > then do not set it. How does it help old guests? Or maybe it's
> > not designed to ...
>
> Indeed. The idea is that QEMU can offer the flag, old guests can reject
> it (or even new guests can reject it, if they decide not to convert into
> secure VMs) and the feature negotiation will succeed with the flag
> unset.
OK. And then what does QEMU do? Assume guest is not encrypted I guess?
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Thiago Jung Bauermann @ 2019-07-15 22:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mike Anderson, Michael Roth, Jean-Philippe Brucker, Jason Wang,
Alexey Kardashevskiy, Ram Pai, linux-kernel, virtualization,
iommu, linuxppc-dev, Christoph Hellwig, David Gibson
In-Reply-To: <20190715163453-mutt-send-email-mst@kernel.org>
Michael S. Tsirkin <mst@redhat.com> writes:
> On Mon, Jul 15, 2019 at 05:29:06PM -0300, Thiago Jung Bauermann wrote:
>>
>> Michael S. Tsirkin <mst@redhat.com> writes:
>>
>> > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
>> >>
>> >>
>> >> Michael S. Tsirkin <mst@redhat.com> writes:
>> >>
>> >> > So this is what I would call this option:
>> >> >
>> >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
>> >> >
>> >> > and the explanation should state that all device
>> >> > addresses are translated by the platform to identical
>> >> > addresses.
>> >> >
>> >> > In fact this option then becomes more, not less restrictive
>> >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
>> >> > by guest to only create identity mappings,
>> >> > and only before driver_ok is set.
>> >> > This option then would always be negotiated together with
>> >> > VIRTIO_F_ACCESS_PLATFORM.
>> >> >
>> >> > Host then must verify that
>> >> > 1. full 1:1 mappings are created before driver_ok
>> >> > or can we make sure this happens before features_ok?
>> >> > that would be ideal as we could require that features_ok fails
>> >> > 2. mappings are not modified between driver_ok and reset
>> >> > i guess attempts to change them will fail -
>> >> > possibly by causing a guest crash
>> >> > or some other kind of platform-specific error
>> >>
>> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring
>> >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
>> >> SLOF as I mentioned above, another is that we would be requiring all
>> >> guests running on the machine (secure guests or not, since we would use
>> >> the same configuration for all guests) to support it. But
>> >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
>> >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about
>> >> it and wouldn't be able to use the device.
>> >
>> > OK and your target is to enable use with kernel drivers within
>> > guests, right?
>>
>> Right.
>>
>> > My question is, we are defining a new flag here, I guess old guests
>> > then do not set it. How does it help old guests? Or maybe it's
>> > not designed to ...
>>
>> Indeed. The idea is that QEMU can offer the flag, old guests can reject
>> it (or even new guests can reject it, if they decide not to convert into
>> secure VMs) and the feature negotiation will succeed with the flag
>> unset.
>
> OK. And then what does QEMU do? Assume guest is not encrypted I guess?
There's nothing different that QEMU needs to do, with or without the
flag. the perspective of the host, a secure guest and a regular guest
work the same way with respect to virtio.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ 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