* Re: [PATCH v9 01/10] namei: obey trailing magic-link DAC permissions
From: Aleksa Sarai @ 2019-07-14 7:11 UTC (permalink / raw)
To: Al Viro
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190712131005.GM17978@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 3530 bytes --]
On 2019-07-12, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Jul 12, 2019 at 10:20:17PM +1000, Aleksa Sarai wrote:
> > On 2019-07-12, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > On Sun, Jul 07, 2019 at 12:57:28AM +1000, Aleksa Sarai wrote:
> > > > @@ -514,7 +516,14 @@ static void set_nameidata(struct nameidata *p, int dfd, struct filename *name)
> > > > p->stack = p->internal;
> > > > p->dfd = dfd;
> > > > p->name = name;
> > > > - p->total_link_count = old ? old->total_link_count : 0;
> > > > + p->total_link_count = 0;
> > > > + p->acc_mode = 0;
> > > > + p->opath_mask = FMODE_PATH_READ | FMODE_PATH_WRITE;
> > > > + if (old) {
> > > > + p->total_link_count = old->total_link_count;
> > > > + p->acc_mode = old->acc_mode;
> > > > + p->opath_mask = old->opath_mask;
> > > > + }
> > >
> > > Huh? Could somebody explain why traversals of NFS4 referrals should inherit
> > > ->acc_mode and ->opath_mask?
> >
> > I'll be honest -- I don't understand what set_nameidata() did so I just
> > did what I thought would be an obvious change (to just copy the
> > contents). I thought it was related to some aspect of the symlink stack
> > handling.
>
> No. It's handling of (very rare) nested pathwalk. The only case I can think
> of is handling of NFS4 referrals - they are triggered by ->d_automount()
> and include NFS4 mount. Which does internal pathwalk of its own, to get
> to the root of subtree being automounted.
>
> NFS has its own recursion protection on that path (no deeper nesting than
> one level of referral traversals), but there some nesting is inevitable;
> we do get another nameidata instance on stack. And for nd_jump_link() we
> need to keep track of the innermost one.
>
> For symlinks nothing of that sort happens - they are dealt with on the same
> struct nameidata. ->total_link_count copying is there for one reason only -
> we want the total amount of symlinks traversed during the pathwalk (including
> the referral processing, etc.) to count towards MAXSYMLINKS check. It could've
> been moved from nameidata to task_struct, but it's cheaper to handle it that
> way.
>
> Again, nesting is *rare*.
Thanks for the explanation, much appreciated. I will drop the old->...
copying hunk.
> > In that case, should they both be set to 0 on set_nameidata()? This will
> > mean that fd re-opening (or magic-link opening) through a
> > set_nameidata() would always fail.
>
> Huh? set_nameidata() is done for *all* instances - it's pretty much the
> constructor of that object (and restore_nameidata() - a destructor).
> Everything goes through it.
Sorry, I meant to drop the copy-from-old logic -- not set it to zero
explicitly in set_nameidata().
> And again, I'm not sure we want these fields in nameidata - IMO they belong
> in open_flags. Things like e.g. stat() don't need them at all.
Yup, I'll work up a version that does the consolidation you mentioned
in your other mail.
> Incidentally, O_PATH opening of symlinks combined with subsequent procfs
> symlink traversals is worth testing - that's where the things get subtle
> and that's where it's easy to get in trouble on modifications.
I have some self-tests of a symlink-to-a-magic-link in the last patch of
the series. Did you mean something even more chained like a symlink to a
/proc/self/fd/$n of an O_NOFOLLOW|O_PATH of a symlink?
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags
From: Aleksa Sarai @ 2019-07-14 7:00 UTC (permalink / raw)
To: Al Viro
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190713024153.GA3817@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 3715 bytes --]
On 2019-07-13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Jul 12, 2019 at 04:00:26PM +0100, Al Viro wrote:
> > On Fri, Jul 12, 2019 at 02:25:53PM +0100, Al Viro wrote:
> >
> > > if (flags & LOOKUP_BENEATH) {
> > > nd->root = nd->path;
> > > if (!(flags & LOOKUP_RCU))
> > > path_get(&nd->root);
> > > else
> > > nd->root_seq = nd->seq;
> >
> > BTW, this assignment is needed for LOOKUP_RCU case. Without it
> > you are pretty much guaranteed that lazy pathwalk will fail,
> > when it comes to complete_walk().
> >
> > Speaking of which, what would happen if LOOKUP_ROOT/LOOKUP_BENEATH
> > combination would someday get passed?
>
> I don't understand what's going on with ->r_seq in there - your
> call of path_is_under() is after having (re-)sampled rename_lock,
> but if that was the only .. in there, who's going to recheck
> the value? For that matter, what's to guarantee that the thing
> won't get moved just as you are returning from handle_dots()?
>
> IOW, what does LOOKUP_IN_ROOT guarantee for caller (openat2())?
I tried to explain this in the commit message for "namei: aggressively
check for nd->root escape on ".." resolution", but I probably could've
explained it better.
The basic property being guaranteed by LOOKUP_IN_ROOT is that it will
not result in resolution of a path component which was not inside the
root of the dirfd tree at some point during resolution (and that all
absolute symlink and ".." resolution will be done relative to the
dirfd). This may smell slightly of chroot(2), because unfortunately it
is a similar concept -- the reason for this is to allow for a more
efficient way to safely resolve paths inside a rootfs than spawning a
separate process to then pass back the fd to the caller.
We don't want to do a path_is_under() check for every ".." (otherwise
lookups have a quadratic slowdown when doing many ".."s), so I instead
only do a check after a rename or a mount (which are the only operations
which could change what ".." points to). And since we do the
path_is_under() check if m_seq or r_seq need a retry, we can re-take
them[+].
The main reason why I don't re-check path_is_under() after handle_dots()
is that there is no way to be sure that a racing rename didn't happen
after your last path_is_under() check. The rename could happen after the
syscall returns, after all.
So, the main purpose of the check is to make sure that a ".."s after a
rename doesn't result in an escape. If the rename happens after we've
traversed through a ".." that means that the ".." was inside the root in
the first place (a root ".." is handled by follow_dotdot). If the rename
happens after we've gone through handle_dots() and there is no
subsequent ".." then to userspace it looks identical to the rename
occurring after the syscall has returned. If there is a subsequent ".."
after a racing rename then we may have moved into a path that wasn't
path_is_under() and so we have to check it.
The only way I could see you could solve the race completely is if you
had a way for userspace to lock things from being able to be renamed (or
MS_MOVE'd). And that feels like a really bad idea to me.
[+]: You asked why don't I re-take m_seq. The reason is that I don't
fully understand all the other m_seq checks being done during
resolution, and we aren't definitely doing them all in
handle_dots(). So I assumed re-taking it could result in me
breaking RCU-walk which obviously would be bad. Since I am the only
thing using nd->r_seq, I can re-take it without issue.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Thiago Jung Bauermann @ 2019-07-14 5:51 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: <20190701092212-mutt-send-email-mst@kernel.org>
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.
> 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 v9 05/10] namei: O_BENEATH-style path resolution flags
From: Al Viro @ 2019-07-14 3:58 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Tetsuo Handa, Alexei Starovoitov,
linux-kernel, David Howells, linux-kselftest, sparclinux,
Shuah Khan, raven, linux-arch, linux-s390, Tycho Andersen, paul,
Aleksa Sarai, linux-arm-kernel, linux-mips, linux-xtensa,
Kees Cook, Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, rgb, linux-api, Chanho Min,
Jeff Layton, Oleg Nesterov, Eric Biederman, linux-alpha,
linux-fsdevel, Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190713024153.GA3817@ZenIV.linux.org.uk>
On Sat, Jul 13, 2019 at 03:41:53AM +0100, Al Viro wrote:
> On Fri, Jul 12, 2019 at 04:00:26PM +0100, Al Viro wrote:
> > On Fri, Jul 12, 2019 at 02:25:53PM +0100, Al Viro wrote:
> >
> > > if (flags & LOOKUP_BENEATH) {
> > > nd->root = nd->path;
> > > if (!(flags & LOOKUP_RCU))
> > > path_get(&nd->root);
> > > else
> > > nd->root_seq = nd->seq;
> >
> > BTW, this assignment is needed for LOOKUP_RCU case. Without it
> > you are pretty much guaranteed that lazy pathwalk will fail,
> > when it comes to complete_walk().
> >
> > Speaking of which, what would happen if LOOKUP_ROOT/LOOKUP_BENEATH
> > combination would someday get passed?
>
> I don't understand what's going on with ->r_seq in there - your
> call of path_is_under() is after having (re-)sampled rename_lock,
> but if that was the only .. in there, who's going to recheck
> the value? For that matter, what's to guarantee that the thing
> won't get moved just as you are returning from handle_dots()?
>
> IOW, what does LOOKUP_IN_ROOT guarantee for caller (openat2())?
Sigh... Usual effects of trying to document things:
1) LOOKUP_NO_EVAL looks bogus. It had been introduced by commit 57d4657716ac
(audit: ignore fcaps on umount) and AFAICS it's crap. It is set in
ksys_umount() and nowhere else. It's ignored by everything except
filename_mountpoint(). The thing is, call graph for filename_mountpoint()
is
filename_mountpoint()
<- user_path_mountpoint_at()
<- ksys_umount()
<- kern_path_mountpoint()
<- autofs_dev_ioctl_ismountpoint()
<- find_autofs_mount()
<- autofs_dev_ioctl_open_mountpoint()
<- autofs_dev_ioctl_requester()
<- autofs_dev_ioctl_ismountpoint()
In other words, that flag is basically "was filename_mountpoint()
been called by umount(2) or has it come from an autofs ioctl?".
And looking at the rationale in that commit, autofs ioctls need
it just as much as umount(2) does. Why is it not set for those
as well? And why is it conditional at all?
1b) ... because audit_inode() wants LOOKUP_... as the last argument,
only to remap it into AUDIT_..., that's why. So audit needs something
guaranteed not to conflict with LOOKUP_PARENT (another flag getting
remapped). So why do we bother with remapping those, anyway? Let's look
at the callers:
fs/namei.c:933: audit_inode(nd->name, nd->stack[0].link.dentry, 0);
fs/namei.c:2353: audit_inode(name, path->dentry, flags & LOOKUP_PARENT);
fs/namei.c:2394: audit_inode(name, parent->dentry, LOOKUP_PARENT);
fs/namei.c:2721: audit_inode(name, path->dentry, flags & LOOKUP_NO_EVAL);
fs/namei.c:3302: audit_inode(nd->name, dir, LOOKUP_PARENT);
fs/namei.c:3336: audit_inode(nd->name, file->f_path.dentry, 0);
fs/namei.c:3371: audit_inode(nd->name, path.dentry, 0);
fs/namei.c:3389: audit_inode(nd->name, nd->path.dentry, 0);
fs/namei.c:3490: audit_inode(nd->name, child, 0);
fs/namei.c:3509: audit_inode(nd->name, path.dentry, 0);
ipc/mqueue.c:788: audit_inode(name, dentry, 0);
In all but two of those we have a nice constant value - 0 or AUDIT_INODE_PARENT.
One of two exceptions is in filename_mountpoint(), and there we want
unconditional AUDIT_INODE_NOEVAL (see above). What of the other? It's
if (likely(!retval))
audit_inode(name, path->dentry, flags & LOOKUP_PARENT);
in filename_lookup(). And that is bogus as well. filename_lookupat() would
better *NOT* get LOOKUP_PARENT in flags. And it doesn't - not since
commit 8bcb77fabd7c (namei: split off filename_lookupat() with LOOKUP_PARENT)
back in 2015. In filename_parentat() introduced there we have
audit_inode(name, parent->dentry, LOOKUP_PARENT);
and at the same point the call in filename_lookupat() should've become
audit_inode(name, path->dentry, 0);
It hadn't; my fault. And after fixing that everything becomes nice and
unconditional - the last argument of audit_inode() is always an AUDIT_...
constant or zero. Moving AUDIT_... definitions outside of ifdef on
CONFIG_AUDITSYSCALL, getting rid of remapping in audit_inode() and
passing the right values in 3 callers that don't pass 0 and LOOKUP_NO_EVAL
can go to hell.
Any objections from audit folks?
2) comment in namei.h is seriously out of sync with reality. To quote:
* - follow links at the end
OK, that's LOOKUP_FOLLOW (1)
* - require a directory
... and LOOKUP_DIRECTORY (2)
* - ending slashes ok even for nonexistent files
... used to be about LOOKUP_CONTINUE (eight years dead now)
* - internal "there are more path components" flag
... LOOKUP_PARENT (16)
* - dentry cache is untrusted; force a real lookup
... LOOKUP_REVAL (32)
* - suppress terminal automount
... used to be LOOKUP_NO_AUTOMOUNT (128), except that it's been
replaced with LOOKUP_AUTOMOUNT (at 4) almost eight years ago. And
the meaning of LOOKUP_AUTOMOUNT is opposite to the comment,
of course.
* - skip revalidation
... LOOKUP_NO_REVAL (128)
* - don't fetch xattrs on audit_inode
... and that's about soon-to-be dead LOOKUP_NO_EVAL (256)
Note that LOOKUP_RCU (at 64) is quietly skipped and so's the tail
of the list. If not for "suppress terminal automount" bit, I wouldn't
really care, but that one makes for a really nasty trap for readers.
I'm going to convert that to (accurate) comments next to actual defines...
3) while looking through LOOKUP_AUTOMOUNT users,
in aa_bind_mount() we have
error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
matching do_loopback(), while tomoyo_mount_acl() has
if (!dev_name || kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
And yes, that *is* hit on mount --bind. As well as on new mounts, where
apparmor (and bdev_lookup()) has plain LOOKUP_FOLLOW.
->sb_mount() is garbage by design (not the least because of the need to
have pathname lookups in the first place, as well as having to duplicate the
demultiplexing parts of do_mount() without fucking it up)...
^ permalink raw reply
* Re: [RFC PATCH kernel] powerpc/xive: Drop deregistered irqs
From: Benjamin Herrenschmidt @ 2019-07-14 1:31 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Greg Kurz, Nicholas Piggin, Paul Mackerras, Cédric Le Goater,
Alistair Popple, David Gibson
In-Reply-To: <9b1fbde4-0884-223e-446c-373f73c366e0@ozlabs.ru>
On Sat, 2019-07-13 at 18:53 +1000, Alexey Kardashevskiy wrote:
>
> On 13/07/2019 09:47, Benjamin Herrenschmidt wrote:
> > On Fri, 2019-07-12 at 19:37 +1000, Alexey Kardashevskiy wrote:
> > >
> >
> >
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/irq.c#n614
> > >
> > > If so, then in order to do EOI, I'll need the desc which is gone,
> > > or
> > > I am missing the point?
> >
> > All you need is drop the local CPU priority.
>
> I know that, the question was how. I cannot use irq_chip in
> arch/powerpc/kernel/irq.c and I do not want to add
> ppc_md.enable_irqs().
Well, best is probably to do just that though, but call it something
like ppc_md.orphan_irq() or something like that instead. Another option
as you mention is to try to scrub queues, but that's trickier to do due
to the lockless nature of the queue handling.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
From: Segher Boessenkool @ 2019-07-13 23:54 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras,
Linux Kernel Mailing List, Nicholas Piggin
In-Reply-To: <CAK7LNASBmZxX+U=LS+dgvet96cA3T6Tf_tiAa2vduUV81DEnBw@mail.gmail.com>
On Sun, Jul 14, 2019 at 07:45:15AM +0900, Masahiro Yamada wrote:
> On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> > > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > > > in a useful way because it is always overridden by the following code
> > > > in the top Makefile:
> > > >
> > > > # use the deterministic mode of AR if available
> > > > KBUILD_ARFLAGS := $(call ar-option,D)
> > > >
> > > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > > > ("kbuild: Fix build with binutils <= 2.19").
> > > >
> > > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > > > beginning.
> > >
> > > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> > >
> > > Is it no longer supported to build a 64-bit kernel with a toolchain
> > > that defaults to 32-bit, or the other way around? And with non-native
> > > toolchains (this one didn't run on Linux, even).
> >
> > It was an --enable-targets=all toolchain, somewhat common for crosses,
> > if that matters.
>
> I always use the same toolchain
> for compile-testing PPC32/64.
>
> I have never been hit by the issue you mention.
> Somebody would have reported it if it were still a problem.
But did you use --enable-targets=all?
The problem was empty archives IIRC. Not a problem anymore with thin
archives, maybe?
> Moreover, commit 43c9127d94d6
> translated the environment variable "GNUTARGET"
> into the command option "--target="
>
> My powerpc-linux-ar does not know it:
>
> powerpc-linux-ar: -t: No such file or directory
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.
Segher
^ permalink raw reply
* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.3-1 tag
From: pr-tracker-bot @ 2019-07-13 23:15 UTC (permalink / raw)
To: Michael Ellerman
Cc: geert+renesas, aik, schwab, stewart, gromero, yamada.masahiro,
oohall, blackgod016574, hch, mikey, nishadkamdar, aneesh.kumar,
yuehaibing, krzk, anju, rostedt, sjitindarsingh, alastair,
naveen.n.rao, nathanl, ravi.bangoria, ajd, geliangtang, groug,
efremov, zhangshaokun, cai, npiggin, dja, info, atrajeev,
chunkeey, malat, gregkh, linuxppc-dev, linux-kernel,
andrew.donnellan, fbarrat, sathnaga, Linus Torvalds
In-Reply-To: <87ims6eey7.fsf@concordia.ellerman.id.au>
The pull request you sent on Sat, 13 Jul 2019 14:28:00 +1000:
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.3-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/192f0f8e9db7efe4ac98d47f5fa4334e43c1204d
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
From: Masahiro Yamada @ 2019-07-13 22:45 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras,
Linux Kernel Mailing List, Nicholas Piggin
In-Reply-To: <20190713131642.GU14074@gate.crashing.org>
On Sat, Jul 13, 2019 at 10:17 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> > On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > > in a useful way because it is always overridden by the following code
> > > in the top Makefile:
> > >
> > > # use the deterministic mode of AR if available
> > > KBUILD_ARFLAGS := $(call ar-option,D)
> > >
> > > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > > ("kbuild: Fix build with binutils <= 2.19").
> > >
> > > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > > beginning.
> >
> > That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
> >
> > Is it no longer supported to build a 64-bit kernel with a toolchain
> > that defaults to 32-bit, or the other way around? And with non-native
> > toolchains (this one didn't run on Linux, even).
>
> It was an --enable-targets=all toolchain, somewhat common for crosses,
> if that matters.
I always use the same toolchain
for compile-testing PPC32/64.
I have never been hit by the issue you mention.
Somebody would have reported it if it were still a problem.
Moreover, commit 43c9127d94d6
translated the environment variable "GNUTARGET"
into the command option "--target="
My powerpc-linux-ar does not know it:
powerpc-linux-ar: -t: No such file or directory
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v4 6/8] KVM: PPC: Ultravisor: Restrict LDBAR access
From: Claudio Carvalho @ 2019-07-13 17:56 UTC (permalink / raw)
To: Ram Pai, Alexey Kardashevskiy
Cc: maddy, Michael Anderson, kvm-ppc, Bharata B Rao, linuxppc-dev,
Ryan Grimm, Sukadev Bhattiprolu, Thiago Bauermann,
Anshuman Khandual
In-Reply-To: <20190701064642.GA5009@ram.ibm.com>
On 7/1/19 3:46 AM, Ram Pai wrote:
> On Mon, Jul 01, 2019 at 04:30:55PM +1000, Alexey Kardashevskiy wrote:
>>
>> On 01/07/2019 16:17, maddy wrote:
>>> On 01/07/19 11:24 AM, Alexey Kardashevskiy wrote:
>>>> On 29/06/2019 06:08, Claudio Carvalho wrote:
>>>>> When the ultravisor firmware is available, it takes control over the
>>>>> LDBAR register. In this case, thread-imc updates and save/restore
>>>>> operations on the LDBAR register are handled by ultravisor.
>>>> What does LDBAR do? "Power ISA™ Version 3.0 B" or "User’s Manual POWER9
>>>> Processor" do not tell.
>>> LDBAR is a per-thread SPR used by thread-imc pmu to dump the counter
>>> data into memory.
>>> LDBAR contains memory address along with few other configuration bits
>>> (it is populated
>>> by the thread-imc pmu driver). It is populated and enabled only when any
>>> of the thread
>>> imc pmu events are monitored.
>>
>> I was actually looking for a spec for this register, what is the
>> document name?
> Its not a architected register. Its documented in the Power9
> workbook.
I also found some information about the LDBAR in
arch/powerpc/perf/imc-pmu.c
Claudio
>
> RP
>
^ permalink raw reply
* Re: [PATCH v4 3/8] KVM: PPC: Ultravisor: Add generic ultravisor call handler
From: Claudio Carvalho @ 2019-07-13 17:42 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu, Thiago Bauermann,
Anshuman Khandual
In-Reply-To: <87lfx4g253.fsf@concordia.ellerman.id.au>
On 7/11/19 9:57 AM, Michael Ellerman wrote:
> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>> From: Ram Pai <linuxram@us.ibm.com>
>>
>> Add the ucall() function, which can be used to make ultravisor calls
>> with varied number of in and out arguments. Ultravisor calls can be made
>> from the host or guests.
>>
>> This copies the implementation of plpar_hcall().
> .. with quite a few changes?
>
> This is one of the things I'd like to see in a Documentation file, so
> that people can review the implementation vs the specification.
I will document this (and other things) in a file under Documentation/powerpc.
>
>> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>> [ Change ucall.S to not save CR, rename and move headers, build ucall.S
>> if CONFIG_PPC_POWERNV set, use R3 for the ucall number and add some
>> comments in the code ]
> Why are we not saving CR? See previous comment about Documentation :)
>
>> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/ultravisor-api.h | 20 +++++++++++++++
>> arch/powerpc/include/asm/ultravisor.h | 20 +++++++++++++++
>> arch/powerpc/kernel/Makefile | 2 +-
>> arch/powerpc/kernel/ucall.S | 30 +++++++++++++++++++++++
>> arch/powerpc/kernel/ultravisor.c | 4 +++
>> 5 files changed, 75 insertions(+), 1 deletion(-)
>> create mode 100644 arch/powerpc/include/asm/ultravisor-api.h
>> create mode 100644 arch/powerpc/kernel/ucall.S
>>
>> diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
>> new file mode 100644
>> index 000000000000..49e766adabc7
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/ultravisor-api.h
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Ultravisor API.
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#ifndef _ASM_POWERPC_ULTRAVISOR_API_H
>> +#define _ASM_POWERPC_ULTRAVISOR_API_H
>> +
>> +#include <asm/hvcall.h>
>> +
>> +/* Return codes */
>> +#define U_NOT_AVAILABLE H_NOT_AVAILABLE
>> +#define U_SUCCESS H_SUCCESS
>> +#define U_FUNCTION H_FUNCTION
>> +#define U_PARAMETER H_PARAMETER
> Is there any benefit in redefining these?
>
>> diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
>> index e5009b0d84ea..a78a2dacfd0b 100644
>> --- a/arch/powerpc/include/asm/ultravisor.h
>> +++ b/arch/powerpc/include/asm/ultravisor.h
>> @@ -8,8 +8,28 @@
>> #ifndef _ASM_POWERPC_ULTRAVISOR_H
>> #define _ASM_POWERPC_ULTRAVISOR_H
>>
>> +#include <asm/ultravisor-api.h>
>> +
>> +#if !defined(__ASSEMBLY__)
> Just #ifndef is fine.
>
>> /* Internal functions */
> How is it internal?
>
>> extern int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
>> int depth, void *data);
>>
>> +/* API functions */
>> +#define UCALL_BUFSIZE 4
> Please don't copy this design from the hcall code, it has led to bugs in
> the past.
>
> See my (still unmerged) attempt to fix this for the hcall case:
> https://patchwork.ozlabs.org/patch/683577/
>
> Basically instead of asking callers nicely to define a certain sized
> buffer, and them forgetting, define a proper type that has the right size.
I will keep that in mind. For now I think we don't need that since the v5
will have ucall_norets() instead.
>
>> +/**
>> + * ucall: Make a powerpc ultravisor call.
>> + * @opcode: The ultravisor call to make.
>> + * @retbuf: Buffer to store up to 4 return arguments in.
>> + *
>> + * This call supports up to 6 arguments and 4 return arguments. Use
>> + * UCALL_BUFSIZE to size the return argument buffer.
>> + */
>> +#if defined(CONFIG_PPC_POWERNV)
> #ifdef
>
>> +long ucall(unsigned long opcode, unsigned long *retbuf, ...);
>> +#endif
>> +
>> +#endif /* !__ASSEMBLY__ */
>> +
>> #endif /* _ASM_POWERPC_ULTRAVISOR_H */
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index f0caa302c8c0..f28baccc0a79 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -154,7 +154,7 @@ endif
>>
>> obj-$(CONFIG_EPAPR_PARAVIRT) += epapr_paravirt.o epapr_hcalls.o
>> obj-$(CONFIG_KVM_GUEST) += kvm.o kvm_emul.o
>> -obj-$(CONFIG_PPC_POWERNV) += ultravisor.o
>> +obj-$(CONFIG_PPC_POWERNV) += ultravisor.o ucall.o
> Same comment about being platforms/powernv ?
>> diff --git a/arch/powerpc/kernel/ucall.S b/arch/powerpc/kernel/ucall.S
>> new file mode 100644
>> index 000000000000..1678f6eb7230
>> --- /dev/null
>> +++ b/arch/powerpc/kernel/ucall.S
>> @@ -0,0 +1,30 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Generic code to perform an ultravisor call.
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#include <asm/ppc_asm.h>
>> +
>> +/*
>> + * This function is based on the plpar_hcall()
> I don't think it meaningfully is any more.
>
>> + */
>> +_GLOBAL_TOC(ucall)
> You don't need the TOC setup here (unless a later patch does?).
>
>> + std r4,STK_PARAM(R4)(r1) /* Save ret buffer */
>> + mr r4,r5
>> + mr r5,r6
>> + mr r6,r7
>> + mr r7,r8
>> + mr r8,r9
>> + mr r9,r10
> Below you space the arguments, here you don't. Pick one or the other please.
>
>> +
>> + sc 2 /* Invoke the ultravisor */
>> +
>> + ld r12,STK_PARAM(R4)(r1)
>> + std r4, 0(r12)
>> + std r5, 8(r12)
>> + std r6, 16(r12)
>> + std r7, 24(r12)
>> +
>> + blr /* Return r3 = status */
>> diff --git a/arch/powerpc/kernel/ultravisor.c b/arch/powerpc/kernel/ultravisor.c
>> index dc6021f63c97..02ddf79a9522 100644
>> --- a/arch/powerpc/kernel/ultravisor.c
>> +++ b/arch/powerpc/kernel/ultravisor.c
>> @@ -8,10 +8,14 @@
>> #include <linux/init.h>
>> #include <linux/printk.h>
>> #include <linux/string.h>
>> +#include <linux/export.h>
>>
>> #include <asm/ultravisor.h>
>> #include <asm/firmware.h>
>>
>> +/* in ucall.S */
>> +EXPORT_SYMBOL_GPL(ucall);
> This should be in ucall.S
Here I'm following the same way that hypercall wrapper symbols are exported.
Last time I tried to export that in ucall.S the linker complained that the
ucall
symbol will not be versioned. Something like this:
https://patchwork.kernel.org/patch/9452759/
Thanks,
Claudio
>
> cheers
^ permalink raw reply
* [PATCH V2] cpufreq: Make cpufreq_generic_init() return void
From: Viresh Kumar @ 2019-07-13 14:36 UTC (permalink / raw)
To: Rafael Wysocki, Markus Mayer, bcm-kernel-feedback-list, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Keguang Zhang, Jiaxun Yang, Kevin Hilman,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Kukjin Kim, Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter
Cc: linux-samsung-soc, Vincent Guittot, linux-pm, Viresh Kumar,
linux-mips, linux-kernel, linux-tegra, linux-omap, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.kumar@linaro.org>
It always returns 0 (success) and its return type should really be void.
Over that, many drivers have added error handling code based on its
return value, which is not required at all.
change its return type to void and update all the callers.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1->V2:
- Fixed compilation warning with s3c64xx driver by replace %d with %ld
(Reported by buildbot).
drivers/cpufreq/bmips-cpufreq.c | 7 ++-----
drivers/cpufreq/cpufreq.c | 4 +---
drivers/cpufreq/davinci-cpufreq.c | 3 ++-
drivers/cpufreq/imx6q-cpufreq.c | 6 ++----
drivers/cpufreq/kirkwood-cpufreq.c | 3 ++-
drivers/cpufreq/loongson1-cpufreq.c | 8 +++-----
drivers/cpufreq/loongson2_cpufreq.c | 3 ++-
drivers/cpufreq/maple-cpufreq.c | 3 ++-
drivers/cpufreq/omap-cpufreq.c | 15 +++++----------
drivers/cpufreq/pasemi-cpufreq.c | 3 ++-
drivers/cpufreq/pmac32-cpufreq.c | 3 ++-
drivers/cpufreq/pmac64-cpufreq.c | 3 ++-
drivers/cpufreq/s3c2416-cpufreq.c | 9 ++-------
drivers/cpufreq/s3c64xx-cpufreq.c | 15 +++------------
drivers/cpufreq/s5pv210-cpufreq.c | 3 ++-
drivers/cpufreq/sa1100-cpufreq.c | 3 ++-
drivers/cpufreq/sa1110-cpufreq.c | 3 ++-
drivers/cpufreq/spear-cpufreq.c | 3 ++-
drivers/cpufreq/tegra20-cpufreq.c | 8 +-------
include/linux/cpufreq.h | 2 +-
20 files changed, 42 insertions(+), 65 deletions(-)
diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c
index 56a4ebbf00e0..2b187d802fe3 100644
--- a/drivers/cpufreq/bmips-cpufreq.c
+++ b/drivers/cpufreq/bmips-cpufreq.c
@@ -141,11 +141,8 @@ static int bmips_cpufreq_init(struct cpufreq_policy *policy)
return ret;
}
- ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
- if (ret)
- bmips_cpufreq_exit(policy);
- else
- pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
+ cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
+ pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
return ret;
}
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 4d6043ee7834..8dda62367816 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -159,7 +159,7 @@ EXPORT_SYMBOL_GPL(arch_set_freq_scale);
* - set policies transition latency
* - policy->cpus with all possible CPUs
*/
-int cpufreq_generic_init(struct cpufreq_policy *policy,
+void cpufreq_generic_init(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table,
unsigned int transition_latency)
{
@@ -171,8 +171,6 @@ int cpufreq_generic_init(struct cpufreq_policy *policy,
* share the clock and voltage and clock.
*/
cpumask_setall(policy->cpus);
-
- return 0;
}
EXPORT_SYMBOL_GPL(cpufreq_generic_init);
diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c
index 3de48ae60c29..297d23cad8b5 100644
--- a/drivers/cpufreq/davinci-cpufreq.c
+++ b/drivers/cpufreq/davinci-cpufreq.c
@@ -90,7 +90,8 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
* Setting the latency to 2000 us to accommodate addition of drivers
* to pre/post change notification list.
*/
- return cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+ cpufreq_generic_init(policy, freq_table, 2000 * 1000);
+ return 0;
}
static struct cpufreq_driver davinci_driver = {
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 47ccfa6b17b7..648a09a1778a 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -190,14 +190,12 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
{
- int ret;
-
policy->clk = clks[ARM].clk;
- ret = cpufreq_generic_init(policy, freq_table, transition_latency);
+ cpufreq_generic_init(policy, freq_table, transition_latency);
policy->suspend_freq = max_freq;
dev_pm_opp_of_register_em(policy->cpus);
- return ret;
+ return 0;
}
static struct cpufreq_driver imx6q_cpufreq_driver = {
diff --git a/drivers/cpufreq/kirkwood-cpufreq.c b/drivers/cpufreq/kirkwood-cpufreq.c
index 7ab564c1f7ae..cb74bdc5baaa 100644
--- a/drivers/cpufreq/kirkwood-cpufreq.c
+++ b/drivers/cpufreq/kirkwood-cpufreq.c
@@ -85,7 +85,8 @@ static int kirkwood_cpufreq_target(struct cpufreq_policy *policy,
/* Module init and exit code */
static int kirkwood_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
- return cpufreq_generic_init(policy, kirkwood_freq_table, 5000);
+ cpufreq_generic_init(policy, kirkwood_freq_table, 5000);
+ return 0;
}
static struct cpufreq_driver kirkwood_cpufreq_driver = {
diff --git a/drivers/cpufreq/loongson1-cpufreq.c b/drivers/cpufreq/loongson1-cpufreq.c
index 21c9ce8526c0..0ea88778882a 100644
--- a/drivers/cpufreq/loongson1-cpufreq.c
+++ b/drivers/cpufreq/loongson1-cpufreq.c
@@ -81,7 +81,7 @@ static int ls1x_cpufreq_init(struct cpufreq_policy *policy)
struct device *cpu_dev = get_cpu_device(policy->cpu);
struct cpufreq_frequency_table *freq_tbl;
unsigned int pll_freq, freq;
- int steps, i, ret;
+ int steps, i;
pll_freq = clk_get_rate(cpufreq->pll_clk) / 1000;
@@ -103,11 +103,9 @@ static int ls1x_cpufreq_init(struct cpufreq_policy *policy)
freq_tbl[i].frequency = CPUFREQ_TABLE_END;
policy->clk = cpufreq->clk;
- ret = cpufreq_generic_init(policy, freq_tbl, 0);
- if (ret)
- kfree(freq_tbl);
+ cpufreq_generic_init(policy, freq_tbl, 0);
- return ret;
+ return 0;
}
static int ls1x_cpufreq_exit(struct cpufreq_policy *policy)
diff --git a/drivers/cpufreq/loongson2_cpufreq.c b/drivers/cpufreq/loongson2_cpufreq.c
index da344696beed..890813e0bb76 100644
--- a/drivers/cpufreq/loongson2_cpufreq.c
+++ b/drivers/cpufreq/loongson2_cpufreq.c
@@ -95,7 +95,8 @@ static int loongson2_cpufreq_cpu_init(struct cpufreq_policy *policy)
}
policy->clk = cpuclk;
- return cpufreq_generic_init(policy, &loongson2_clockmod_table[0], 0);
+ cpufreq_generic_init(policy, &loongson2_clockmod_table[0], 0);
+ return 0;
}
static int loongson2_cpufreq_exit(struct cpufreq_policy *policy)
diff --git a/drivers/cpufreq/maple-cpufreq.c b/drivers/cpufreq/maple-cpufreq.c
index f5220b3d4ec5..28d346062166 100644
--- a/drivers/cpufreq/maple-cpufreq.c
+++ b/drivers/cpufreq/maple-cpufreq.c
@@ -140,7 +140,8 @@ static unsigned int maple_cpufreq_get_speed(unsigned int cpu)
static int maple_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
- return cpufreq_generic_init(policy, maple_cpu_freqs, 12000);
+ cpufreq_generic_init(policy, maple_cpu_freqs, 12000);
+ return 0;
}
static struct cpufreq_driver maple_cpufreq_driver = {
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index 29643f06a3c3..8d14b42a8c6f 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -122,23 +122,18 @@ static int omap_cpu_init(struct cpufreq_policy *policy)
dev_err(mpu_dev,
"%s: cpu%d: failed creating freq table[%d]\n",
__func__, policy->cpu, result);
- goto fail;
+ clk_put(policy->clk);
+ return result;
}
}
atomic_inc_return(&freq_table_users);
/* FIXME: what's the actual transition time? */
- result = cpufreq_generic_init(policy, freq_table, 300 * 1000);
- if (!result) {
- dev_pm_opp_of_register_em(policy->cpus);
- return 0;
- }
+ cpufreq_generic_init(policy, freq_table, 300 * 1000);
+ dev_pm_opp_of_register_em(policy->cpus);
- freq_table_free();
-fail:
- clk_put(policy->clk);
- return result;
+ return 0;
}
static int omap_cpu_exit(struct cpufreq_policy *policy)
diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
index 6b1e4abe3248..93f39a1d4c3d 100644
--- a/drivers/cpufreq/pasemi-cpufreq.c
+++ b/drivers/cpufreq/pasemi-cpufreq.c
@@ -196,7 +196,8 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
policy->cur = pas_freqs[cur_astate].frequency;
ppc_proc_freq = policy->cur * 1000ul;
- return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
+ cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency());
+ return 0;
out_unmap_sdcpwr:
iounmap(sdcpwr_mapbase);
diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c
index 650104d729f3..73621bc11976 100644
--- a/drivers/cpufreq/pmac32-cpufreq.c
+++ b/drivers/cpufreq/pmac32-cpufreq.c
@@ -372,7 +372,8 @@ static int pmac_cpufreq_target( struct cpufreq_policy *policy,
static int pmac_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
- return cpufreq_generic_init(policy, pmac_cpu_freqs, transition_latency);
+ cpufreq_generic_init(policy, pmac_cpu_freqs, transition_latency);
+ return 0;
}
static u32 read_gpio(struct device_node *np)
diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c
index 1af3492a000d..d7542a106e6b 100644
--- a/drivers/cpufreq/pmac64-cpufreq.c
+++ b/drivers/cpufreq/pmac64-cpufreq.c
@@ -321,7 +321,8 @@ static unsigned int g5_cpufreq_get_speed(unsigned int cpu)
static int g5_cpufreq_cpu_init(struct cpufreq_policy *policy)
{
- return cpufreq_generic_init(policy, g5_cpu_freqs, transition_latency);
+ cpufreq_generic_init(policy, g5_cpu_freqs, transition_latency);
+ return 0;
}
static struct cpufreq_driver g5_cpufreq_driver = {
diff --git a/drivers/cpufreq/s3c2416-cpufreq.c b/drivers/cpufreq/s3c2416-cpufreq.c
index f7ff1ed7fef1..106910351c41 100644
--- a/drivers/cpufreq/s3c2416-cpufreq.c
+++ b/drivers/cpufreq/s3c2416-cpufreq.c
@@ -447,21 +447,16 @@ static int s3c2416_cpufreq_driver_init(struct cpufreq_policy *policy)
/* Datasheet says PLL stabalisation time must be at least 300us,
* so but add some fudge. (reference in LOCKCON0 register description)
*/
- ret = cpufreq_generic_init(policy, s3c_freq->freq_table,
+ cpufreq_generic_init(policy, s3c_freq->freq_table,
(500 * 1000) + s3c_freq->regulator_latency);
- if (ret)
- goto err_freq_table;
-
register_reboot_notifier(&s3c2416_cpufreq_reboot_notifier);
return 0;
-err_freq_table:
#ifdef CONFIG_ARM_S3C2416_CPUFREQ_VCORESCALE
- regulator_put(s3c_freq->vddarm);
err_vddarm:
-#endif
clk_put(s3c_freq->armclk);
+#endif
err_armclk:
clk_put(s3c_freq->hclk);
err_hclk:
diff --git a/drivers/cpufreq/s3c64xx-cpufreq.c b/drivers/cpufreq/s3c64xx-cpufreq.c
index 37df2d892eb0..af0c00dabb22 100644
--- a/drivers/cpufreq/s3c64xx-cpufreq.c
+++ b/drivers/cpufreq/s3c64xx-cpufreq.c
@@ -144,7 +144,6 @@ static void s3c64xx_cpufreq_config_regulator(void)
static int s3c64xx_cpufreq_driver_init(struct cpufreq_policy *policy)
{
- int ret;
struct cpufreq_frequency_table *freq;
if (policy->cpu != 0)
@@ -165,8 +164,7 @@ static int s3c64xx_cpufreq_driver_init(struct cpufreq_policy *policy)
#ifdef CONFIG_REGULATOR
vddarm = regulator_get(NULL, "vddarm");
if (IS_ERR(vddarm)) {
- ret = PTR_ERR(vddarm);
- pr_err("Failed to obtain VDDARM: %d\n", ret);
+ pr_err("Failed to obtain VDDARM: %ld\n", PTR_ERR(vddarm));
pr_err("Only frequency scaling available\n");
vddarm = NULL;
} else {
@@ -196,16 +194,9 @@ static int s3c64xx_cpufreq_driver_init(struct cpufreq_policy *policy)
* the PLLs, which we don't currently) is ~300us worst case,
* but add some fudge.
*/
- ret = cpufreq_generic_init(policy, s3c64xx_freq_table,
+ cpufreq_generic_init(policy, s3c64xx_freq_table,
(500 * 1000) + regulator_latency);
- if (ret != 0) {
- pr_err("Failed to configure frequency table: %d\n",
- ret);
- regulator_put(vddarm);
- clk_put(policy->clk);
- }
-
- return ret;
+ return 0;
}
static struct cpufreq_driver s3c64xx_cpufreq_driver = {
diff --git a/drivers/cpufreq/s5pv210-cpufreq.c b/drivers/cpufreq/s5pv210-cpufreq.c
index e5cb17d4be7b..5d10030f2560 100644
--- a/drivers/cpufreq/s5pv210-cpufreq.c
+++ b/drivers/cpufreq/s5pv210-cpufreq.c
@@ -541,7 +541,8 @@ static int s5pv210_cpu_init(struct cpufreq_policy *policy)
s5pv210_dram_conf[1].freq = clk_get_rate(dmc1_clk);
policy->suspend_freq = SLEEP_FREQ;
- return cpufreq_generic_init(policy, s5pv210_freq_table, 40000);
+ cpufreq_generic_init(policy, s5pv210_freq_table, 40000);
+ return 0;
out_dmc1:
clk_put(dmc0_clk);
diff --git a/drivers/cpufreq/sa1100-cpufreq.c b/drivers/cpufreq/sa1100-cpufreq.c
index ab5cab93e638..5c075ef6adc0 100644
--- a/drivers/cpufreq/sa1100-cpufreq.c
+++ b/drivers/cpufreq/sa1100-cpufreq.c
@@ -181,7 +181,8 @@ static int sa1100_target(struct cpufreq_policy *policy, unsigned int ppcr)
static int __init sa1100_cpu_init(struct cpufreq_policy *policy)
{
- return cpufreq_generic_init(policy, sa11x0_freq_table, 0);
+ cpufreq_generic_init(policy, sa11x0_freq_table, 0);
+ return 0;
}
static struct cpufreq_driver sa1100_driver __refdata = {
diff --git a/drivers/cpufreq/sa1110-cpufreq.c b/drivers/cpufreq/sa1110-cpufreq.c
index dab54e051c0e..d9d04d935b3a 100644
--- a/drivers/cpufreq/sa1110-cpufreq.c
+++ b/drivers/cpufreq/sa1110-cpufreq.c
@@ -303,7 +303,8 @@ static int sa1110_target(struct cpufreq_policy *policy, unsigned int ppcr)
static int __init sa1110_cpu_init(struct cpufreq_policy *policy)
{
- return cpufreq_generic_init(policy, sa11x0_freq_table, 0);
+ cpufreq_generic_init(policy, sa11x0_freq_table, 0);
+ return 0;
}
/* sa1110_driver needs __refdata because it must remain after init registers
diff --git a/drivers/cpufreq/spear-cpufreq.c b/drivers/cpufreq/spear-cpufreq.c
index 4074e2615522..73bd8dc47074 100644
--- a/drivers/cpufreq/spear-cpufreq.c
+++ b/drivers/cpufreq/spear-cpufreq.c
@@ -153,8 +153,9 @@ static int spear_cpufreq_target(struct cpufreq_policy *policy,
static int spear_cpufreq_init(struct cpufreq_policy *policy)
{
policy->clk = spear_cpufreq.clk;
- return cpufreq_generic_init(policy, spear_cpufreq.freq_tbl,
+ cpufreq_generic_init(policy, spear_cpufreq.freq_tbl,
spear_cpufreq.transition_latency);
+ return 0;
}
static struct cpufreq_driver spear_cpufreq_driver = {
diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
index 3c32cc7b0671..f84ecd22f488 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -118,17 +118,11 @@ static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
static int tegra_cpu_init(struct cpufreq_policy *policy)
{
struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
- int ret;
clk_prepare_enable(cpufreq->cpu_clk);
/* FIXME: what's the actual transition time? */
- ret = cpufreq_generic_init(policy, freq_table, 300 * 1000);
- if (ret) {
- clk_disable_unprepare(cpufreq->cpu_clk);
- return ret;
- }
-
+ cpufreq_generic_init(policy, freq_table, 300 * 1000);
policy->clk = cpufreq->cpu_clk;
policy->suspend_freq = freq_table[0].frequency;
return 0;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index d757a56a74dc..536a049d7ecc 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -992,7 +992,7 @@ extern struct freq_attr *cpufreq_generic_attr[];
int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy);
unsigned int cpufreq_generic_get(unsigned int cpu);
-int cpufreq_generic_init(struct cpufreq_policy *policy,
+void cpufreq_generic_init(struct cpufreq_policy *policy,
struct cpufreq_frequency_table *table,
unsigned int transition_latency);
#endif /* _LINUX_CPUFREQ_H */
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related
* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
From: Segher Boessenkool @ 2019-07-13 13:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Stephen Rothwell, linuxppc-dev, Paul Mackerras, linux-kernel,
Nicholas Piggin
In-Reply-To: <20190713124744.GS14074@gate.crashing.org>
On Sat, Jul 13, 2019 at 07:47:44AM -0500, Segher Boessenkool wrote:
> On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> > The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> > in a useful way because it is always overridden by the following code
> > in the top Makefile:
> >
> > # use the deterministic mode of AR if available
> > KBUILD_ARFLAGS := $(call ar-option,D)
> >
> > The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> > ("kbuild: Fix build with binutils <= 2.19").
> >
> > The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> > beginning.
>
> That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
>
> Is it no longer supported to build a 64-bit kernel with a toolchain
> that defaults to 32-bit, or the other way around? And with non-native
> toolchains (this one didn't run on Linux, even).
It was an --enable-targets=all toolchain, somewhat common for crosses,
if that matters.
Segher
^ permalink raw reply
* Re: [PATCH] powerpc: remove meaningless KBUILD_ARFLAGS addition
From: Segher Boessenkool @ 2019-07-13 12:47 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Stephen Rothwell, linux-kernel, Nicholas Piggin, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20190713032106.8509-1-yamada.masahiro@socionext.com>
On Sat, Jul 13, 2019 at 12:21:06PM +0900, Masahiro Yamada wrote:
> The KBUILD_ARFLAGS addition in arch/powerpc/Makefile has never worked
> in a useful way because it is always overridden by the following code
> in the top Makefile:
>
> # use the deterministic mode of AR if available
> KBUILD_ARFLAGS := $(call ar-option,D)
>
> The code in the top Makefile was added in 2011, by commit 40df759e2b9e
> ("kbuild: Fix build with binutils <= 2.19").
>
> The KBUILD_ARFLAGS addition for ppc has always been dead code from the
> beginning.
That was added in 43c9127d94d6 to replace my 8995ac870273 from 2007.
Is it no longer supported to build a 64-bit kernel with a toolchain
that defaults to 32-bit, or the other way around? And with non-native
toolchains (this one didn't run on Linux, even).
Segher
^ permalink raw reply
* Re: [RFC PATCH kernel] powerpc/xive: Drop deregistered irqs
From: Alexey Kardashevskiy @ 2019-07-13 8:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev
Cc: Greg Kurz, Nicholas Piggin, Paul Mackerras, Cédric Le Goater,
Alistair Popple, David Gibson
In-Reply-To: <d735f761b3d1cc1cd6df5fd566c75cae661fe4e7.camel@kernel.crashing.org>
On 13/07/2019 09:47, Benjamin Herrenschmidt wrote:
> On Fri, 2019-07-12 at 19:37 +1000, Alexey Kardashevskiy wrote:
>>
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/kernel/irq.c#n614
>>
>> If so, then in order to do EOI, I'll need the desc which is gone, or
>> I am missing the point?
>
> All you need is drop the local CPU priority.
I know that, the question was how. I cannot use irq_chip in
arch/powerpc/kernel/irq.c and I do not want to add ppc_md.enable_irqs().
--
Alexey
^ permalink raw reply
* Re: [PATCH v4 2/8] powerpc: Introduce FW_FEATURE_ULTRAVISOR
From: Claudio Carvalho @ 2019-07-12 18:01 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu, Thiago Bauermann,
Anshuman Khandual
In-Reply-To: <87k1cog250.fsf@concordia.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]
On 7/11/19 9:57 AM, Michael Ellerman wrote:
> Claudio Carvalho <cclaudio@linux.ibm.com> writes:
>> diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
>> new file mode 100644
>> index 000000000000..e5009b0d84ea
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/ultravisor.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Ultravisor definitions
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#ifndef _ASM_POWERPC_ULTRAVISOR_H
>> +#define _ASM_POWERPC_ULTRAVISOR_H
>> +
>> +/* Internal functions */
>> +extern int early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
>> + int depth, void *data);
> Please don't use extern in new headers.
>
>> diff --git a/arch/powerpc/kernel/ultravisor.c b/arch/powerpc/kernel/ultravisor.c
>> new file mode 100644
>> index 000000000000..dc6021f63c97
>> --- /dev/null
>> +++ b/arch/powerpc/kernel/ultravisor.c
> Is there a reason this (and other later files) aren't in platforms/powernv ?
Yes, there is.
https://www.spinics.net/lists/kvm-ppc/msg14998.html
We also need to do ucalls from a secure guest and its kernel may not have CONFIG_PPC_POWERNV=y. I can make it clear in the commit message.
>
>> @@ -0,0 +1,24 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Ultravisor high level interfaces
>> + *
>> + * Copyright 2019, IBM Corporation.
>> + *
>> + */
>> +#include <linux/init.h>
>> +#include <linux/printk.h>
>> +#include <linux/string.h>
>> +
>> +#include <asm/ultravisor.h>
>> +#include <asm/firmware.h>
>> +
>> +int __init early_init_dt_scan_ultravisor(unsigned long node, const char *uname,
>> + int depth, void *data)
>> +{
>> + if (depth != 1 || strcmp(uname, "ibm,ultravisor") != 0)
>> + return 0;
> I know you're following the example of OPAL, but this is not the best
> way to search for the ultravisor node.
>
> It makes the location and name of the node part of the ABI, when there's
> no need for it to be.
>
> If instead you just scan the tree for a node that is *compatible* with
> "ibm,ultravisor" (or whatever compatible string) then the node can be
> placed any where in the tree and have any name, which gives us the most
> flexibility in future to change the location of the device tree node.
I will do that in the next version.
Thanks,
Claudio
>
> cheers
>
[-- Attachment #2: Type: text/html, Size: 3943 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Halil Pasic @ 2019-07-12 15:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, iommu, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-fsdevel, Thomas Gleixner, linuxppc-dev,
Alexey Dobriyan, Thiago Jung Bauermann, Marek Szyprowski
In-Reply-To: <20190712151129.GA30636@lst.de>
On Fri, 12 Jul 2019 17:11:29 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Fri, Jul 12, 2019 at 04:51:53PM +0200, Halil Pasic wrote:
> > Thank you very much! I will have another look, but it seems to me,
> > without further measures taken, this would break protected virtualization
> > support on s390. The effect of the che for s390 is that
> > force_dma_unencrypted() will always return false instead calling into
> > the platform code like it did before the patch, right?
> >
> > Should I send a Fixes: e67a5ed1f86f "dma-direct: Force unencrypted DMA
> > under SME for certain DMA masks" (Tom Lendacky, 2019-07-10) patch that
> > rectifies things for s390 or how do we want handle this?
>
> Yes, please do. I hadn't noticed the s390 support had landed in
> mainline already.
>
Will do! I guess I should do the patch against the for-next branch of the
dma-mapping tree. But that branch does not have the s390 support patches (yet?).
To fix it I need both e67a5ed1f86f and 64e1f0c531d1 "s390/mm: force
swiotlb for protected virtualization" (Halil Pasic, 2018-09-13). Or
should I wait for e67a5ed1f86f landing in mainline?
Regards,
Halil
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Halil Pasic @ 2019-07-12 14:51 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, iommu, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, linux-fsdevel, Thomas Gleixner, linuxppc-dev,
Alexey Dobriyan, Thiago Jung Bauermann, Marek Szyprowski
In-Reply-To: <20190712140812.GA29628@lst.de>
On Fri, 12 Jul 2019 16:08:12 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Fri, Jul 12, 2019 at 03:09:12PM +0200, Halil Pasic wrote:
> > This is the implementation for the guys that don't
> > have ARCH_HAS_MEM_ENCRYPT.
> >
> > Means sev_active() may not be used in such code after this
> > patch. What about
> >
> > static inline bool force_dma_unencrypted(void)
> > {
> > return sev_active();
> > }
> >
> > in kernel/dma/direct.c?
>
> FYI, I have this pending in the dma-mapping tree:
>
> http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/e67a5ed1f86f4370991c601f2fcad9ebf9e1eebb
Thank you very much! I will have another look, but it seems to me,
without further measures taken, this would break protected virtualization
support on s390. The effect of the che for s390 is that
force_dma_unencrypted() will always return false instead calling into
the platform code like it did before the patch, right?
Should I send a Fixes: e67a5ed1f86f "dma-direct: Force unencrypted DMA
under SME for certain DMA masks" (Tom Lendacky, 2019-07-10) patch that
rectifies things for s390 or how do we want handle this?
Regards,
Halil
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Halil Pasic @ 2019-07-12 13:09 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
linuxppc-dev, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20190712053631.9814-4-bauerman@linux.ibm.com>
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).
>
> 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.
>
> diff --git a/arch/x86/kernel/crash_dump_64.c b/arch/x86/kernel/crash_dump_64.c
> index 22369dd5de3b..045e82e8945b 100644
> --- a/arch/x86/kernel/crash_dump_64.c
> +++ b/arch/x86/kernel/crash_dump_64.c
> @@ -70,3 +70,8 @@ ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize,
> {
> return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true);
> }
> +
> +ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> +{
> + return read_from_oldmem(buf, count, ppos, 0, sev_active());
> +}
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 57957c91c6df..ca1f20bedd8c 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -100,9 +100,9 @@ static int pfn_is_ram(unsigned long pfn)
> }
>
> /* Reads a page from the oldmem device from given offset. */
> -static ssize_t read_from_oldmem(char *buf, size_t count,
> - u64 *ppos, int userbuf,
> - bool encrypted)
> +ssize_t read_from_oldmem(char *buf, size_t count,
> + u64 *ppos, int userbuf,
> + bool encrypted)
> {
> unsigned long pfn, offset;
> size_t nr_bytes;
> @@ -166,7 +166,7 @@ void __weak elfcorehdr_free(unsigned long long addr)
> */
> ssize_t __weak elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> {
> - return read_from_oldmem(buf, count, ppos, 0, sev_active());
> + return read_from_oldmem(buf, count, ppos, 0, false);
> }
>
> /*
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index f774c5eb9e3c..4664fc1871de 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -115,4 +115,18 @@ static inline int vmcore_add_device_dump(struct vmcoredd_data *data)
> return -EOPNOTSUPP;
> }
> #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
> +
> +#ifdef CONFIG_PROC_VMCORE
> +ssize_t read_from_oldmem(char *buf, size_t count,
> + u64 *ppos, int userbuf,
> + bool encrypted);
> +#else
> +static inline ssize_t read_from_oldmem(char *buf, size_t count,
> + u64 *ppos, int userbuf,
> + bool encrypted)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_PROC_VMCORE */
> +
> #endif /* LINUX_CRASHDUMP_H */
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index f2e399fb626b..a3747fcae466 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -21,7 +21,6 @@
>
> #else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
>
> -static inline bool sev_active(void) { return false; }
This is the implementation for the guys that don't
have ARCH_HAS_MEM_ENCRYPT.
Means sev_active() may not be used in such code after this
patch. What about
static inline bool force_dma_unencrypted(void)
{
return sev_active();
}
in kernel/dma/direct.c?
Regards,
Halil
> static inline bool mem_encrypt_active(void) { return false; }
>
> #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Christoph Hellwig @ 2019-07-13 8:08 UTC (permalink / raw)
To: Halil Pasic
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
linuxppc-dev, Christoph Hellwig, Thiago Jung Bauermann,
Marek Szyprowski
In-Reply-To: <20190712174249.33b74535.pasic@linux.ibm.com>
On Fri, Jul 12, 2019 at 05:42:49PM +0200, Halil Pasic wrote:
>
> Will do! I guess I should do the patch against the for-next branch of the
> dma-mapping tree. But that branch does not have the s390 support patches (yet?).
> To fix it I need both e67a5ed1f86f and 64e1f0c531d1 "s390/mm: force
> swiotlb for protected virtualization" (Halil Pasic, 2018-09-13). Or
> should I wait for e67a5ed1f86f landing in mainline?
I've rebased the dma-mapping for-next branch to latest mainline as of
today that has both commits.
^ permalink raw reply
* Re: [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
From: Christoph Hellwig @ 2019-07-13 7:29 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <20190713044554.28719-3-bauerman@linux.ibm.com>
While this looks generally good to me, I think we want to split this
into three patches:
1) update the swiotlb printk
2) removing the dma-mapping check and printk
3) clean up the mem_encrypt.h interface.
^ permalink raw reply
* Re: [PATCH 1/3] x86,s390: Move ARCH_HAS_MEM_ENCRYPT definition to arch/Kconfig
From: Christoph Hellwig @ 2019-07-13 7:27 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
Ingo Molnar, Borislav Petkov, Thomas Lendacky, H. Peter Anvin,
linux-fsdevel, Thomas Gleixner, linuxppc-dev, Christoph Hellwig,
Marek Szyprowski
In-Reply-To: <20190713044554.28719-2-bauerman@linux.ibm.com>
On Sat, Jul 13, 2019 at 01:45:52AM -0300, Thiago Jung Bauermann wrote:
> powerpc is also going to use this feature, so put it in a generic location.
Looks good,
even without a third arch using it we should never habe symbols defined
under arch/$(ARCH) that are used in common code to start with.
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* [PATCH v2 13/13] powerpc/configs: Enable secure guest support in pseries and ppc64 defconfigs
From: Thiago Jung Bauermann @ 2019-07-13 6:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
linux-kernel, Claudio Carvalho, Ryan Grimm, Paul Mackerras,
Christoph Hellwig, Thiago Jung Bauermann
In-Reply-To: <20190713060023.8479-1-bauerman@linux.ibm.com>
From: Ryan Grimm <grimm@linux.vnet.ibm.com>
Enables running as a secure guest in platforms with an Ultravisor.
Signed-off-by: Ryan Grimm <grimm@linux.vnet.ibm.com>
Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/configs/ppc64_defconfig | 1 +
arch/powerpc/configs/pseries_defconfig | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig
index c63b019ba753..0b04c5ca9e13 100644
--- a/arch/powerpc/configs/ppc64_defconfig
+++ b/arch/powerpc/configs/ppc64_defconfig
@@ -31,6 +31,7 @@ CONFIG_DTL=y
CONFIG_SCANLOG=m
CONFIG_PPC_SMLPAR=y
CONFIG_IBMEBUS=y
+CONFIG_PPC_SVM=y
CONFIG_PPC_MAPLE=y
CONFIG_PPC_PASEMI=y
CONFIG_PPC_PASEMI_IOMMU=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index 38abc9c1770a..26126b4d4de3 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -42,6 +42,7 @@ CONFIG_DTL=y
CONFIG_SCANLOG=m
CONFIG_PPC_SMLPAR=y
CONFIG_IBMEBUS=y
+CONFIG_PPC_SVM=y
# CONFIG_PPC_PMAC is not set
CONFIG_RTAS_FLASH=m
CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
^ permalink raw reply related
* [PATCH v2 12/13] powerpc/pseries/svm: Force SWIOTLB for secure guests
From: Thiago Jung Bauermann @ 2019-07-13 6:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
linux-kernel, Claudio Carvalho, Paul Mackerras, Christoph Hellwig,
Thiago Jung Bauermann, Anshuman Khandual
In-Reply-To: <20190713060023.8479-1-bauerman@linux.ibm.com>
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
SWIOTLB checks range of incoming CPU addresses to be bounced and sees if
the device can access it through its DMA window without requiring bouncing.
In such cases it just chooses to skip bouncing. But for cases like secure
guests on powerpc platform all addresses need to be bounced into the shared
pool of memory because the host cannot access it otherwise. Hence the need
to do the bouncing is not related to device's DMA window and use of bounce
buffers is forced by setting swiotlb_force.
Also, connect the shared memory conversion functions into the
ARCH_HAS_MEM_ENCRYPT hooks and call swiotlb_update_mem_attributes() to
convert SWIOTLB's memory pool to shared memory.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
[ bauerman: Use ARCH_HAS_MEM_ENCRYPT hooks to share swiotlb memory pool. ]
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/include/asm/mem_encrypt.h | 26 +++++++++++++++
arch/powerpc/platforms/pseries/Kconfig | 3 ++
arch/powerpc/platforms/pseries/svm.c | 45 ++++++++++++++++++++++++++
3 files changed, 74 insertions(+)
diff --git a/arch/powerpc/include/asm/mem_encrypt.h b/arch/powerpc/include/asm/mem_encrypt.h
new file mode 100644
index 000000000000..d8d5a7fcf298
--- /dev/null
+++ b/arch/powerpc/include/asm/mem_encrypt.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * SVM helper functions
+ *
+ * Copyright 2019 IBM Corporation
+ */
+
+#ifndef _ASM_POWERPC_MEM_ENCRYPT_H
+#define _ASM_POWERPC_MEM_ENCRYPT_H
+
+#include <asm/svm.h>
+
+static inline bool mem_encrypt_active(void)
+{
+ return is_secure_guest();
+}
+
+static inline bool force_dma_unencrypted(struct device *dev)
+{
+ return is_secure_guest();
+}
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* _ASM_POWERPC_MEM_ENCRYPT_H */
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index d09deb05bb66..9e35cddddf73 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -149,6 +149,9 @@ config PAPR_SCM
config PPC_SVM
bool "Secure virtual machine (SVM) support for POWER"
depends on PPC_PSERIES
+ select SWIOTLB
+ select ARCH_HAS_MEM_ENCRYPT
+ select ARCH_HAS_FORCE_DMA_UNENCRYPTED
help
There are certain POWER platforms which support secure guests using
the Protected Execution Facility, with the help of an Ultravisor
diff --git a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c
index c508196f7c83..618622d636d5 100644
--- a/arch/powerpc/platforms/pseries/svm.c
+++ b/arch/powerpc/platforms/pseries/svm.c
@@ -7,8 +7,53 @@
*/
#include <linux/mm.h>
+#include <asm/machdep.h>
+#include <asm/svm.h>
+#include <asm/swiotlb.h>
#include <asm/ultravisor.h>
+static int __init init_svm(void)
+{
+ if (!is_secure_guest())
+ return 0;
+
+ /* Don't release the SWIOTLB buffer. */
+ ppc_swiotlb_enable = 1;
+
+ /*
+ * Since the guest memory is inaccessible to the host, devices always
+ * need to use the SWIOTLB buffer for DMA even if dma_capable() says
+ * otherwise.
+ */
+ swiotlb_force = SWIOTLB_FORCE;
+
+ /* Share the SWIOTLB buffer with the host. */
+ swiotlb_update_mem_attributes();
+
+ return 0;
+}
+machine_early_initcall(pseries, init_svm);
+
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ if (!PAGE_ALIGNED(addr))
+ return -EINVAL;
+
+ uv_unshare_page(PHYS_PFN(__pa(addr)), numpages);
+
+ return 0;
+}
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ if (!PAGE_ALIGNED(addr))
+ return -EINVAL;
+
+ uv_share_page(PHYS_PFN(__pa(addr)), numpages);
+
+ return 0;
+}
+
/* There's one dispatch log per CPU. */
#define NR_DTL_PAGE (DISPATCH_LOG_BYTES * CONFIG_NR_CPUS / PAGE_SIZE)
^ permalink raw reply related
* [PATCH v2 11/13] powerpc/pseries/iommu: Don't use dma_iommu_ops on secure guests
From: Thiago Jung Bauermann @ 2019-07-13 6:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
linux-kernel, Claudio Carvalho, Paul Mackerras, Christoph Hellwig,
Thiago Jung Bauermann
In-Reply-To: <20190713060023.8479-1-bauerman@linux.ibm.com>
Secure guest memory is inacessible to devices so regular DMA isn't
possible.
In that case set devices' dma_map_ops to NULL so that the generic
DMA code path will use SWIOTLB and DMA to bounce buffers.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/platforms/pseries/iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 03bbb299320e..7d9550edb700 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -50,6 +50,7 @@
#include <asm/udbg.h>
#include <asm/mmzone.h>
#include <asm/plpar_wrappers.h>
+#include <asm/svm.h>
#include "pseries.h"
@@ -1332,7 +1333,10 @@ void iommu_init_early_pSeries(void)
of_reconfig_notifier_register(&iommu_reconfig_nb);
register_memory_notifier(&iommu_mem_nb);
- set_pci_dma_ops(&dma_iommu_ops);
+ if (is_secure_guest())
+ set_pci_dma_ops(NULL);
+ else
+ set_pci_dma_ops(&dma_iommu_ops);
}
static int __init disable_multitce(char *str)
^ permalink raw reply related
* [PATCH v2 10/13] powerpc/pseries/svm: Disable doorbells in SVM guests
From: Thiago Jung Bauermann @ 2019-07-13 6:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
linux-kernel, Claudio Carvalho, Paul Mackerras,
Sukadev Bhattiprolu, Christoph Hellwig, Thiago Jung Bauermann
In-Reply-To: <20190713060023.8479-1-bauerman@linux.ibm.com>
From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
Normally, the HV emulates some instructions like MSGSNDP, MSGCLRP
from a KVM guest. To emulate the instructions, it must first read
the instruction from the guest's memory and decode its parameters.
However for a secure guest (aka SVM), the page containing the
instruction is in secure memory and the HV cannot access directly.
It would need the Ultravisor (UV) to facilitate accessing the
instruction and parameters but the UV currently does not have
the support for such accesses.
Until the UV has such support, disable doorbells in SVMs. This might
incur a performance hit but that is yet to be quantified.
With this patch applied (needed only in SVMs not needed for HV) we
are able to launch SVM guests with multi-core support. Eg:
qemu -smp sockets=2,cores=2,threads=2.
Fix suggested by Benjamin Herrenschmidt. Thanks to input from
Paul Mackerras, Ram Pai and Michael Anderson.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/platforms/pseries/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 3df46123cce3..95a5c24a1544 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -45,6 +45,7 @@
#include <asm/dbell.h>
#include <asm/plpar_wrappers.h>
#include <asm/code-patching.h>
+#include <asm/svm.h>
#include "pseries.h"
#include "offline_states.h"
@@ -225,7 +226,7 @@ static __init void pSeries_smp_probe_xics(void)
{
xics_smp_probe();
- if (cpu_has_feature(CPU_FTR_DBELL))
+ if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
smp_ops->cause_ipi = smp_pseries_cause_ipi;
else
smp_ops->cause_ipi = icp_ops->cause_ipi;
^ permalink raw reply related
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