Re: [PATCH 3/6] intel_sgx: driver for Intel Secure Guard eXtensions
On 25-04-16 10:34, Jarkko Sakkinen wrote: > diff --git a/drivers/staging/intel_sgx/isgx_ioctl.c b/drivers/staging/intel_sgx/isgx_ioctl.c > new file mode 100644 > index 000..9d8b36b > --- /dev/null > +++ b/drivers/staging/intel_sgx/isgx_ioctl.c > > +static long isgx_ioctl_enclave_create(struct file *filep, unsigned int cmd, > + unsigned long arg) > > + secs->base = vm_mmap(filep, 0, secs->size, > + PROT_READ | PROT_WRITE | PROT_EXEC, > + MAP_SHARED, 0); Why does the ioctl interface map userspace memory for an open device? There's already a perfectly good syscall for that: mmap. > diff --git a/drivers/staging/intel_sgx/isgx_user.h > b/drivers/staging/intel_sgx/isgx_user.h > new file mode 100644 > index 000..672d19c > --- /dev/null > +++ b/drivers/staging/intel_sgx/isgx_user.h > > +#define SGX_ADD_SKIP_EEXTEND 0x1 > + > +struct sgx_add_param { > + unsigned long addr; > + unsigned long user_addr; > + struct isgx_secinfo *secinfo; > + unsigned intflags; > +}; The hardware supports calling EEXTEND on only a part of a page, I think the driver should also support that. Jethro ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v8 net-next 1/1] hv_sock: introduce Hyper-V Sockets
> From: Cathy Avery [mailto:cav...@redhat.com] > Sent: Wednesday, April 27, 2016 0:19 > To: Dexuan Cui ; gre...@linuxfoundation.org; > da...@davemloft.net; net...@vger.kernel.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; Jason Wang > ; KY Srinivasan ; Haiyang Zhang > ; vkuzn...@redhat.com; j...@perches.com > Subject: Re: [PATCH v8 net-next 1/1] hv_sock: introduce Hyper-V Sockets > > Hi, > > I will be working with Dexuan to possibly port this functionality into RHEL. > > Here are my initial comments. Mostly stylistic. They are prefaced by CAA. > > Cathy Avery Thank you very much, Cathy! I'll take your pretty good suggestions and post a new version. Thanks, -- Dexuan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Tue, Apr 26, 2016 at 2:52 PM, Pavel Machek wrote: > On Tue 2016-04-26 21:59:52, One Thousand Gnomes wrote: >> > But... that will mean that my ssh will need to be SGX-aware, and that >> > I will not be able to switch to AMD machine in future. ... or to other >> > Intel machine for that matter, right? >> >> I'm not privy to AMD's CPU design plans. >> >> However I think for the ssl/ssh case you'd use the same interfaces >> currently available for plugging in TPMs and dongles. It's a solved >> problem in the crypto libraries. >> >> > What new syscalls would be needed for ssh to get all this support? >> >> I don't see why you'd need new syscalls. > > So the kernel will implement few selected crypto algorithms, similar > to what TPM would provide, using SGX, and then userspace no longer > needs to know about SGX? No, other way around. The kernel will provide a basic interface to SGX and userspace can do whatever it wants. If userspace wants to use RSA, userspace will provide an actual RSA implementation, in more or less normal x86 binary form, and will map it into user addresses. It will tell the kernel "hey, this address range is an enclave", and the kernel will set it up as such and tell the CPU about it. Userspace will then use SGX instructions to communicate with the enclave. It's pretty neat, and it's completely agnostic to the purpose of the enclave. --Andy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Apr 26, 2016 1:11 PM, "Pavel Machek" wrote: > > Hi! > > > >> >> The firmware uses PRMRR registers to reserve an area of physical > > >> >> memory > > >> >> called Enclave Page Cache (EPC). There is a hardware unit in the > > >> >> processor called Memory Encryption Engine. The MEE encrypts and > > >> >> decrypts > > >> >> the EPC pages as they enter and leave the processor package. > > >> > > > >> > What are non-evil use cases for this? > > >> > > >> Storing your ssh private key encrypted such that even someone who > > >> completely compromises your system can't get the actual private key > > > > > > Well, if someone gets root on my system, he can get my ssh private > > > key right? > > > > > > So, you can use this to prevent "cold boot" attacks? (You know, > > > stealing machine, liquid nitrogen, moving DIMMs to different machine > > > to read them?) Ok. That's non-evil. > > > > Preventing cold boot attacks is really just icing on the cake. The > > real point of this is to allow you to run an "enclave". An SGX > > enclave has unencrypted code but gets access to a key that only it can > > access. It could use that key to unwrap your ssh private key and sign > > with it without ever revealing the unwrapped key. No one, not even > > root, can read enclave memory once the enclave is initialized and gets > > access to its personalized key. The point of the memory encryption > > engine to to prevent even cold boot attacks from being used to read > > enclave memory. > > Ok, so the attacker can still access the "other" machine, but ok, key > is protected. > > But... that will mean that my ssh will need to be SGX-aware, and that > I will not be able to switch to AMD machine in future. ... or to other > Intel machine for that matter, right? That's the whole point. You could keep an unwrapped copy of the key offline so you could provision another machine if needed. > > What new syscalls would be needed for ssh to get all this support? This patchset or similar, plus some user code and an enclave to use. Sadly, on current CPUs, you also need Intel to bless the enclave. It looks like new CPUs might relax that requirement. > > > > Is there reason not to enable this for whole RAM if the hw can do it? > > > > The HW can't, at least not in the current implementation. Also, the > > metadata has considerable overhead (no clue whether there's a > > performance hit, but there's certainly a memory usage hit). > > :-(. > > > >> out. Using this in conjunction with an RPMB device to make it Rather > > >> Difficult (tm) for third parties to decrypt your disk even if you > > >> password has low entropy. There are plenty more. > > > > > > I'm not sure what RPMB is, but I don't think you can make it too hard > > > to decrypt my disk if my password has low entropy. ... And I don't see > > > how encrypting RAM helps there. > > > > Replay Protected Memory Block. It's a device that allows someone to > > write to it and confirm that the write happened and the old contents > > is no longer available. You could use it to implement an enclave that > > checks a password for your disk but only allows you to try a certain > > number of times. > > Ookay... I guess I can get a fake Replay Protected Memory block, which > will confirm that write happened and not do anything from China, but > ok, if you put that memory on the CPU, you raise the bar to a "rather > difficult" (tm) level. Nice. It's not so easy for the RPMB to leak things. It would be much easier for it to simply not provide replay protection (i.e. more or less what the FBI asked from Apple: keep allowing guesses even though that shouldn't work). > > But that also means that when my CPU dies, I'll no longer be able to > access the encrypted data. You could implement your own escrow policy and keep a copy in the safe. > > And, again, it means that quite complex new kernel-user interface will > be needed, right? It's actually fairly straightforward, and the kernel part doesn't care what you use it for (the kernel part is the same for disk encryption and ssh, for example, except that disk encryption would care about replay protection, whereas ssh wouldn't). --Andy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Tue 2016-04-26 21:59:52, One Thousand Gnomes wrote: > > But... that will mean that my ssh will need to be SGX-aware, and that > > I will not be able to switch to AMD machine in future. ... or to other > > Intel machine for that matter, right? > > I'm not privy to AMD's CPU design plans. > > However I think for the ssl/ssh case you'd use the same interfaces > currently available for plugging in TPMs and dongles. It's a solved > problem in the crypto libraries. > > > What new syscalls would be needed for ssh to get all this support? > > I don't see why you'd need new syscalls. So the kernel will implement few selected crypto algorithms, similar to what TPM would provide, using SGX, and then userspace no longer needs to know about SGX? Ok, I guess that's simple. It also means it is boring, and the multiuser-game-of-the-day will not be able to protect the (plain text) password from the cold boot attack. Nor will be emacs be able to protect in-memory copy of my diary from cold boot attack. So I guess yes, some new syscalls would be nice :-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
> But... that will mean that my ssh will need to be SGX-aware, and that > I will not be able to switch to AMD machine in future. ... or to other > Intel machine for that matter, right? I'm not privy to AMD's CPU design plans. However I think for the ssl/ssh case you'd use the same interfaces currently available for plugging in TPMs and dongles. It's a solved problem in the crypto libraries. > What new syscalls would be needed for ssh to get all this support? I don't see why you'd need new syscalls. > Ookay... I guess I can get a fake Replay Protected Memory block, which > will confirm that write happened and not do anything from China, but It's not quite that simple because there are keys and a counter involved but I am sure doable. > And, again, it means that quite complex new kernel-user interface will > be needed, right? Why ? For user space we have perfectly good existing system calls, for kernel space we have existing interfaces to the crypto and key layers for modules to use. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
> > Storing your ssh private key encrypted such that even someone who > > completely compromises your system can't get the actual private key > > Well, if someone gets root on my system, he can get my ssh private > key right? Potentially not. If you are using a TPM or other TEE (such as SGX) they can't because the authentication is done from within the TEE. They may be able to hack your box and use the TEE to login somewhere but not to get the key out. Stopping the latter requires a TEE with its own secure input keypad (like some of the USB dongles) Other uses might be things like keeping a copy of the rpm database so you can ask the TEE if the database you have right now happens to match the one you signed as authentic. I suspect there are lots of interesting things that can be done with dm_crypt and also IMA in this area too. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
> Replay Protected Memory Block. It's a device that allows someone to > write to it and confirm that the write happened and the old contents > is no longer available. You could use it to implement an enclave that > checks a password for your disk but only allows you to try a certain > number of times. rpmb is found in a load of hardware today notably MMC/SD cards. Android phones often use it to store sensitive system data. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
Hi! > >> >> The firmware uses PRMRR registers to reserve an area of physical memory > >> >> called Enclave Page Cache (EPC). There is a hardware unit in the > >> >> processor called Memory Encryption Engine. The MEE encrypts and decrypts > >> >> the EPC pages as they enter and leave the processor package. > >> > > >> > What are non-evil use cases for this? > >> > >> Storing your ssh private key encrypted such that even someone who > >> completely compromises your system can't get the actual private key > > > > Well, if someone gets root on my system, he can get my ssh private > > key right? > > > > So, you can use this to prevent "cold boot" attacks? (You know, > > stealing machine, liquid nitrogen, moving DIMMs to different machine > > to read them?) Ok. That's non-evil. > > Preventing cold boot attacks is really just icing on the cake. The > real point of this is to allow you to run an "enclave". An SGX > enclave has unencrypted code but gets access to a key that only it can > access. It could use that key to unwrap your ssh private key and sign > with it without ever revealing the unwrapped key. No one, not even > root, can read enclave memory once the enclave is initialized and gets > access to its personalized key. The point of the memory encryption > engine to to prevent even cold boot attacks from being used to read > enclave memory. Ok, so the attacker can still access the "other" machine, but ok, key is protected. But... that will mean that my ssh will need to be SGX-aware, and that I will not be able to switch to AMD machine in future. ... or to other Intel machine for that matter, right? What new syscalls would be needed for ssh to get all this support? > > Is there reason not to enable this for whole RAM if the hw can do it? > > The HW can't, at least not in the current implementation. Also, the > metadata has considerable overhead (no clue whether there's a > performance hit, but there's certainly a memory usage hit). :-(. > >> out. Using this in conjunction with an RPMB device to make it Rather > >> Difficult (tm) for third parties to decrypt your disk even if you > >> password has low entropy. There are plenty more. > > > > I'm not sure what RPMB is, but I don't think you can make it too hard > > to decrypt my disk if my password has low entropy. ... And I don't see > > how encrypting RAM helps there. > > Replay Protected Memory Block. It's a device that allows someone to > write to it and confirm that the write happened and the old contents > is no longer available. You could use it to implement an enclave that > checks a password for your disk but only allows you to try a certain > number of times. Ookay... I guess I can get a fake Replay Protected Memory block, which will confirm that write happened and not do anything from China, but ok, if you put that memory on the CPU, you raise the bar to a "rather difficult" (tm) level. Nice. But that also means that when my CPU dies, I'll no longer be able to access the encrypted data. And, again, it means that quite complex new kernel-user interface will be needed, right? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Tue, Apr 26, 2016 at 12:41 PM, Pavel Machek wrote: > On Tue 2016-04-26 12:05:48, Andy Lutomirski wrote: >> On Tue, Apr 26, 2016 at 12:00 PM, Pavel Machek wrote: >> > On Mon 2016-04-25 20:34:07, Jarkko Sakkinen wrote: >> >> Intel(R) SGX is a set of CPU instructions that can be used by >> >> applications to set aside private regions of code and data. The code >> >> outside the enclave is disallowed to access the memory inside the >> >> enclave by the CPU access control. >> >> >> >> The firmware uses PRMRR registers to reserve an area of physical memory >> >> called Enclave Page Cache (EPC). There is a hardware unit in the >> >> processor called Memory Encryption Engine. The MEE encrypts and decrypts >> >> the EPC pages as they enter and leave the processor package. >> > >> > What are non-evil use cases for this? >> >> Storing your ssh private key encrypted such that even someone who >> completely compromises your system can't get the actual private key > > Well, if someone gets root on my system, he can get my ssh private > key right? > > So, you can use this to prevent "cold boot" attacks? (You know, > stealing machine, liquid nitrogen, moving DIMMs to different machine > to read them?) Ok. That's non-evil. Preventing cold boot attacks is really just icing on the cake. The real point of this is to allow you to run an "enclave". An SGX enclave has unencrypted code but gets access to a key that only it can access. It could use that key to unwrap your ssh private key and sign with it without ever revealing the unwrapped key. No one, not even root, can read enclave memory once the enclave is initialized and gets access to its personalized key. The point of the memory encryption engine to to prevent even cold boot attacks from being used to read enclave memory. This could probably be used for evil, but I think the evil uses are outweighed by the good uses. > > Is there reason not to enable this for whole RAM if the hw can do it? The HW can't, at least not in the current implementation. Also, the metadata has considerable overhead (no clue whether there's a performance hit, but there's certainly a memory usage hit). > >> out. Using this in conjunction with an RPMB device to make it Rather >> Difficult (tm) for third parties to decrypt your disk even if you >> password has low entropy. There are plenty more. > > I'm not sure what RPMB is, but I don't think you can make it too hard > to decrypt my disk if my password has low entropy. ... And I don't see > how encrypting RAM helps there. Replay Protected Memory Block. It's a device that allows someone to write to it and confirm that the write happened and the old contents is no longer available. You could use it to implement an enclave that checks a password for your disk but only allows you to try a certain number of times. There are some hints in the whitepapers that such a mechanism might be present on existing Skylake chipsets. I'm not really sure. > > Pavel > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- Andy Lutomirski AMA Capital Management, LLC ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Tue 2016-04-26 12:05:48, Andy Lutomirski wrote: > On Tue, Apr 26, 2016 at 12:00 PM, Pavel Machek wrote: > > On Mon 2016-04-25 20:34:07, Jarkko Sakkinen wrote: > >> Intel(R) SGX is a set of CPU instructions that can be used by > >> applications to set aside private regions of code and data. The code > >> outside the enclave is disallowed to access the memory inside the > >> enclave by the CPU access control. > >> > >> The firmware uses PRMRR registers to reserve an area of physical memory > >> called Enclave Page Cache (EPC). There is a hardware unit in the > >> processor called Memory Encryption Engine. The MEE encrypts and decrypts > >> the EPC pages as they enter and leave the processor package. > > > > What are non-evil use cases for this? > > Storing your ssh private key encrypted such that even someone who > completely compromises your system can't get the actual private key Well, if someone gets root on my system, he can get my ssh private key right? So, you can use this to prevent "cold boot" attacks? (You know, stealing machine, liquid nitrogen, moving DIMMs to different machine to read them?) Ok. That's non-evil. Is there reason not to enable this for whole RAM if the hw can do it? > out. Using this in conjunction with an RPMB device to make it Rather > Difficult (tm) for third parties to decrypt your disk even if you > password has low entropy. There are plenty more. I'm not sure what RPMB is, but I don't think you can make it too hard to decrypt my disk if my password has low entropy. ... And I don't see how encrypting RAM helps there. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Mon 2016-04-25 20:34:07, Jarkko Sakkinen wrote: > Intel(R) SGX is a set of CPU instructions that can be used by > applications to set aside private regions of code and data. The code > outside the enclave is disallowed to access the memory inside the > enclave by the CPU access control. > > The firmware uses PRMRR registers to reserve an area of physical memory > called Enclave Page Cache (EPC). There is a hardware unit in the > processor called Memory Encryption Engine. The MEE encrypts and decrypts > the EPC pages as they enter and leave the processor package. What are non-evil use cases for this? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/6] Intel Secure Guard Extensions
On Tue, Apr 26, 2016 at 12:00 PM, Pavel Machek wrote: > On Mon 2016-04-25 20:34:07, Jarkko Sakkinen wrote: >> Intel(R) SGX is a set of CPU instructions that can be used by >> applications to set aside private regions of code and data. The code >> outside the enclave is disallowed to access the memory inside the >> enclave by the CPU access control. >> >> The firmware uses PRMRR registers to reserve an area of physical memory >> called Enclave Page Cache (EPC). There is a hardware unit in the >> processor called Memory Encryption Engine. The MEE encrypts and decrypts >> the EPC pages as they enter and leave the processor package. > > What are non-evil use cases for this? Storing your ssh private key encrypted such that even someone who completely compromises your system can't get the actual private key out. Using this in conjunction with an RPMB device to make it Rather Difficult (tm) for third parties to decrypt your disk even if you password has low entropy. There are plenty more. Think of this as the first time that a secure enclave will be widely available that anyone can program themselves. Well, almost: Skylake doesn't actually permit that for, ahem, reasons. But if you read the very most recent SDM update, it would appear that future Intel CPUs will allow this as long as the firmware doesn't get in the way. Look for IA32_SGXLEHASHSIG (possibly misspelled slightly -- the first few letters are correct). --Andy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v8 net-next 1/1] hv_sock: introduce Hyper-V Sockets
Hi, I will be working with Dexuan to possibly port this functionality into RHEL. Here are my initial comments. Mostly stylistic. They are prefaced by CAA. Thanks, Cathy Avery On 04/07/2016 09:36 PM, Dexuan Cui wrote: Hyper-V Sockets (hv_sock) supplies a byte-stream based communication mechanism between the host and the guest. It's somewhat like TCP over VMBus, but the transportation layer (VMBus) is much simpler than IP. With Hyper-V Sockets, applications between the host and the guest can talk to each other directly by the traditional BSD-style socket APIs. Hyper-V Sockets is only available on new Windows hosts, like Windows Server 2016. More info is in this article "Make your own integration services": https://msdn.microsoft.com/en-us/virtualization/hyperv_on_windows/develop/make_mgmt_service The patch implements the necessary support in the guest side by introducing a new socket address family AF_HYPERV. Signed-off-by: Dexuan Cui Cc: "K. Y. Srinivasan" Cc: Haiyang Zhang Cc: Vitaly Kuznetsov --- MAINTAINERS |2 + include/linux/hyperv.h | 16 + include/linux/socket.h |5 +- include/net/af_hvsock.h | 51 ++ include/uapi/linux/hyperv.h | 25 + net/Kconfig |1 + net/Makefile|1 + net/hv_sock/Kconfig | 10 + net/hv_sock/Makefile|3 + net/hv_sock/af_hvsock.c | 1483 +++ 10 files changed, 1595 insertions(+), 2 deletions(-) create mode 100644 include/net/af_hvsock.h create mode 100644 net/hv_sock/Kconfig create mode 100644 net/hv_sock/Makefile create mode 100644 net/hv_sock/af_hvsock.c diff --git a/MAINTAINERS b/MAINTAINERS index 67d99dd..7b6f203 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5267,7 +5267,9 @@ F:drivers/pci/host/pci-hyperv.c F:drivers/net/hyperv/ F:drivers/scsi/storvsc_drv.c F:drivers/video/fbdev/hyperv_fb.c +F: net/hv_sock/ F:include/linux/hyperv.h +F: include/net/af_hvsock.h F:tools/hv/ F:Documentation/ABI/stable/sysfs-bus-vmbus diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index aa0fadc..b92439d 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1338,4 +1338,20 @@ extern __u32 vmbus_proto_version; int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id, const uuid_le *shv_host_servie_id); +struct vmpipe_proto_header { + u32 pkt_type; + u32 data_size; +} __packed; + +#define HVSOCK_HEADER_LEN (sizeof(struct vmpacket_descriptor) + \ +sizeof(struct vmpipe_proto_header)) + +/* See 'prev_indices' in hv_ringbuffer_read(), hv_ringbuffer_write() */ +#define PREV_INDICES_LEN (sizeof(u64)) + +#define HVSOCK_PKT_LEN(payload_len)(HVSOCK_HEADER_LEN + \ + ALIGN((payload_len), 8) + \ + PREV_INDICES_LEN) +#define HVSOCK_MIN_PKT_LEN HVSOCK_PKT_LEN(1) + #endif /* _HYPERV_H */ diff --git a/include/linux/socket.h b/include/linux/socket.h index 73bf6c6..88b1ccd 100644 --- a/include/linux/socket.h +++ b/include/linux/socket.h @@ -201,8 +201,8 @@ struct ucred { #define AF_NFC39 /* NFC sockets */ #define AF_VSOCK 40 /* vSockets */ #define AF_KCM41 /* Kernel Connection Multiplexor*/ - -#define AF_MAX 42 /* For now.. */ +#define AF_HYPERV 42 /* Hyper-V Sockets */ +#define AF_MAX 43 /* For now.. */ /* Protocol families, same as address families. */ #define PF_UNSPEC AF_UNSPEC @@ -249,6 +249,7 @@ struct ucred { #define PF_NFCAF_NFC #define PF_VSOCK AF_VSOCK #define PF_KCMAF_KCM +#define PF_HYPERV AF_HYPERV #define PF_MAXAF_MAX /* Maximum queue length specifiable by listen. */ diff --git a/include/net/af_hvsock.h b/include/net/af_hvsock.h new file mode 100644 index 000..a5aa28d --- /dev/null +++ b/include/net/af_hvsock.h @@ -0,0 +1,51 @@ +#ifndef __AF_HVSOCK_H__ +#define __AF_HVSOCK_H__ + +#include +#include +#include + +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV (5 * PAGE_SIZE) +#define VMBUS_RINGBUFFER_SIZE_HVSOCK_SEND (5 * PAGE_SIZE) + +#define HVSOCK_RCV_BUF_SZ VMBUS_RINGBUFFER_SIZE_HVSOCK_RECV +#define HVSOCK_SND_BUF_SZ PAGE_SIZE + +#define sk_to_hvsock(__sk)((struct hvsock_sock *)(__sk)) +#define hvsock_to_sk(__hvsk) ((struct sock *)(__hvsk)) + +struct hvsock_sock { + /* sk must be the first member. */ + struct sock sk; + + struct sockaddr_hv local_addr; + struct sockaddr_hv remote_addr; + + /* protected by the global hvsock_mutex */ + struct list_head bound_list; + struct list_head connected_list; + + struct list_head accept_queue; + /* used by enqueue and de
[PATCH] Staging: speakup: i18n: Fixed a CHECK coding style.
NULL comparison has been changed to correct coding style. --- drivers/staging/speakup/i18n.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c index 8960079..a031a2d 100644 --- a/drivers/staging/speakup/i18n.c +++ b/drivers/staging/speakup/i18n.c @@ -407,9 +407,9 @@ static char *next_specifier(char *input) int found = 0; char *next_percent = input; - while ((next_percent != NULL) && !found) { + while (next_percent && !found) { next_percent = strchr(next_percent, '%'); - if (next_percent != NULL) { + if (next_percent) { /* skip over doubled percent signs */ while ((next_percent[0] == '%') && (next_percent[1] == '%')) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v13 2/2] staging/android: refactor SYNC IOCTLs
From: Gustavo Padovan Change SYNC_IOC_FILE_INFO (former SYNC_IOC_FENCE_INFO) behaviour to avoid future API breaks and optimize buffer allocation. Now num_fences can be filled by the caller to inform how many fences it wants to retrieve from the kernel. If the num_fences passed is greater than zero info->sync_fence_info should point to a buffer with enough space to fit all fences. However if num_fences passed to the kernel is 0, the kernel will reply with number of fences of the sync_file. Sending first an ioctl with num_fences = 0 can optimize buffer allocation, in a first call with num_fences = 0 userspace will receive the actual number of fences in the num_fences filed. Then it can allocate a buffer with the correct size on sync_fence_info and call SYNC_IOC_FILE_INFO again, but now with the actual value of num_fences in the sync_file. info->sync_fence_info was converted to __u64 pointer to prevent 32bit compatibility issues. And a flags member was added. An example userspace code for the later would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); info.flags = 0; err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { info.flags = 0; size = sizeof(struct sync_fence_info) * num_fences; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } Finally the IOCTLs numbers were changed to avoid any potential old userspace running the old API to get weird errors. Changing the opcodes will make them fail right away. This is just a precaution, there no upstream users of these interfaces yet and the only user is Android, but we don't expect anyone trying to run android userspace and all it dependencies on top of upstream kernels. Signed-off-by: Gustavo Padovan Reviewed-by: Maarten Lankhorst Acked-by: Greg Hackmann Acked-by: Rob Clark Acked-by: Daniel Vetter --- v2: fix fence_info memory leak v3: Comments from Emil Velikov - improve commit message - remove __u64 cast - remove check for output fields in file_info - clean up sync_fill_fence_info() Comments from Maarten Lankhorst - remove in.num_fences && !in.sync_fence_info check - remove info->len and use only num_fences to calculate size Comments from Dan Carpenter - fix info->sync_fence_info documentation v4: remove allocated struct sync_file_info (comment from Maarten) v5: merge all commits that were changing the ABI v6: fix -Wint-to-pointer-cast error on info.sync_fence_info --- drivers/staging/android/sync.c | 76 - drivers/staging/android/uapi/sync.h | 36 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 3a8f210..f9c6094 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, goto err_put_fd; } + if (data.flags || data.pad) { + err = -EINVAL; + goto err_put_fd; + } + fence2 = sync_file_fdget(data.fd2); if (!fence2) { err = -ENOENT; @@ -479,13 +484,9 @@ err_put_fd: return err; } -static int sync_fill_fence_info(struct fence *fence, void *data, int size) +static void sync_fill_fence_info(struct fence *fence, + struct sync_fence_info *info) { - struct sync_fence_info *info = data; - - if (size < sizeof(*info)) - return -ENOMEM; - strlcpy(info->obj_name, fence->ops->get_timeline_name(fence), sizeof(info->obj_name)); strlcpy(info->driver_name, fence->ops->get_driver_name(fence), @@ -495,58 +496,63 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) else info->status = 0; info->timestamp_ns = ktime_to_ns(fence->timestamp); - - return sizeof(*info); } static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info info; + struct sync_fence_info *fence_info = NULL; __u32 size; - __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) + if (info.flags || info.pad) return -EINVAL; - if (size > 4096) -
[PATCH v13 0/2] staging/android: Sync ABI rework
From: Gustavo Padovan Hi Greg, This patchset clean up the Sync ABI and then improve in to a more optimized version. Also it is now less likely to need changes in the future. This is not breaking any upstream user of the sync framework, as no driver wired support for it, so far Android is the only user. A patch to AOSP will be provided to fix it there. We've made the changes in a way that userspace can figure out if the new versions are present and if not fallback to the older ABI version. More information on patch 2 description. To accomplish that we had to create a u64_to_user_ptr() macro so patch 1 adds that and also fixes some places in the kernel that were using (void __user *)(uintptr_t) cast directly. It already has Acks from maintainers of drm drivers it changes. Patch 2 is the actual rework and has Ack from the people interested in it, including Android folks. Gustavo Padovan (2): kernel.h: add u64_to_user_ptr() staging/android: refactor SYNC IOCTLs drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 ++-- drivers/gpu/drm/i915/i915_drv.h | 5 -- drivers/gpu/drm/i915/i915_gem.c | 14 ++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 ++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 ++-- drivers/staging/android/sync.c | 76 +++- drivers/staging/android/uapi/sync.h | 36 + include/linux/kernel.h | 7 +++ 8 files changed, 94 insertions(+), 80 deletions(-) -- 2.5.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v13 1/2] kernel.h: add u64_to_user_ptr()
From: Gustavo Padovan This function had copies in 3 different files. Unify them in kernel.h. Cc: Joe Perches Cc: Andrew Morton Cc: David Airlie Cc: Daniel Vetter Cc: Rob Clark Signed-off-by: Gustavo Padovan Acked-by: Daniel Vetter[drm/i915/] Acked-by: Rob Clark[drm/msm/] Acked-by: Lucas Stach [drm/etinav/] Acked-by: Maarten Lankhorst --- v2: add typecheck() (comment from Maarten Lankhorst) v3: make u64_to_user_ptr() a macro (comment from Joe Perches) v4: collect all Acked-bys --- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 5 - drivers/gpu/drm/i915/i915_gem.c | 14 +++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 14 +++--- drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++ include/linux/kernel.h | 7 +++ 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 236ada9..afdd55d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -28,11 +28,6 @@ #define BO_LOCKED 0x4000 #define BO_PINNED 0x2000 -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static struct etnaviv_gem_submit *submit_create(struct drm_device *dev, struct etnaviv_gpu *gpu, size_t nr) { @@ -347,21 +342,21 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data, cmdbuf->exec_state = args->exec_state; cmdbuf->ctx = file->driver_priv; - ret = copy_from_user(bos, to_user_ptr(args->bos), + ret = copy_from_user(bos, u64_to_user_ptr(args->bos), args->nr_bos * sizeof(*bos)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(relocs, to_user_ptr(args->relocs), + ret = copy_from_user(relocs, u64_to_user_ptr(args->relocs), args->nr_relocs * sizeof(*relocs)); if (ret) { ret = -EFAULT; goto err_submit_cmds; } - ret = copy_from_user(stream, to_user_ptr(args->stream), + ret = copy_from_user(stream, u64_to_user_ptr(args->stream), args->stream_size); if (ret) { ret = -EFAULT; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1048093..bb624cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3576,11 +3576,6 @@ static inline i915_reg_t i915_vgacntrl_reg(struct drm_device *dev) return VGACNTRL; } -static inline void __user *to_user_ptr(u64 address) -{ - return (void __user *)(uintptr_t)address; -} - static inline unsigned long msecs_to_jiffies_timeout(const unsigned int m) { unsigned long j = msecs_to_jiffies(m); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dabc089..2889716 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -324,7 +324,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; void *vaddr = obj->phys_handle->vaddr + args->offset; - char __user *user_data = to_user_ptr(args->data_ptr); + char __user *user_data = u64_to_user_ptr(args->data_ptr); int ret = 0; /* We manually control the domain here and pretend that it @@ -605,7 +605,7 @@ i915_gem_shmem_pread(struct drm_device *dev, int needs_clflush = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -692,7 +692,7 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return 0; if (!access_ok(VERIFY_WRITE, - to_user_ptr(args->data_ptr), + u64_to_user_ptr(args->data_ptr), args->size)) return -EFAULT; @@ -783,7 +783,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, if (ret) goto out_unpin; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; offset = i915_gem_obj_ggtt_offset(obj) + args->offset; @@ -907,7 +907,7 @@ i915_gem_shmem_pwrite(struct drm_device *dev, int needs_clflush_before = 0; struct sg_page_iter sg_iter; - user_data = to_user_ptr(args->data_ptr); + user_data = u64_to_user_ptr(args->data_ptr); remain = args->size; obj_do_bit17_swizzling = i915_gem_object_needs_bit17_swizzle(obj); @@ -1036,12 +1036,12 @@ i915_g
Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis
On 26/04/16 15:47, Parth Sane wrote: > Hi, > Yes you are right on that regard. I did do that in my previous patches. > It’s just something that I will have to remember now onwards. Thanks. > Regards, > Parth Sane Sure. It isn't a big issue, just thought it was worth mentioning as a tip. Please keep submitting fixes. Appreciated. Luis >> On 26-Apr-2016, at 8:14 PM, Luis de Bethencourt >> wrote: >> >> On 26/04/16 15:33, Parth Sane wrote: >>> Hi, >>> Thanks for the feedback. I did find this issue with the assistance >>> checkpatch.pl script. >>> Regards, >>> Parth Sane >> >> Normally people mention the checkpatch warning they are fixing. >> >> For example: >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5f10ef7dce02d8e36665974aca0977cda58c1122 >> >> Verbosity in the commit message is a good thing :) >> >> I have tested that your patch still applies cleanly to Greg's >> staging-testing branch. >> >> Thanks, >> Luis >> On 26-Apr-2016, at 8:00 PM, Luis de Bethencourt wrote: On 25/04/16 16:43, Parth Sane wrote: > Fixed alignment to match open parenthesis. > > Signed-off-by: Parth Sane > > --- > Changes in v6: > -Added line before Signed-off message > This last version looks good to me. Did you find this issue with checkpatch.pl? Thanks, Luis >>> > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 1/8] dma-buf/fence: add fence_collection fences
On Mon, Apr 25, 2016 at 07:33:21PM -0300, Gustavo Padovan wrote: > +static const char *fence_collection_get_timeline_name(struct fence *fence) > +{ > + return "no context"; "unbound" to distinguish from fence contexts within a timeline? > +static bool fence_collection_enable_signaling(struct fence *fence) > +{ > + struct fence_collection *collection = to_fence_collection(fence); > + int i; > + > + for (i = 0 ; i < collection->num_fences ; i++) { > + if (fence_add_callback(collection->fences[i].fence, > +&collection->fences[i].cb, > +collection_check_cb_func)) { > + atomic_dec(&collection->num_pending_fences); > + return false; Don't stop, we need to enable all the others! > + } > + } > + > + return !!atomic_read(&collection->num_pending_fences); Redundant !! > +} > + > +static bool fence_collection_signaled(struct fence *fence) > +{ > + struct fence_collection *collection = to_fence_collection(fence); > + > + return (atomic_read(&collection->num_pending_fences) == 0); Redundant () > +static signed long fence_collection_wait(struct fence *fence, bool intr, > + signed long timeout) > +{ What advantage does this have over fence_default_wait? You enable signaling on all, then wait sequentially. The code looks redundant and could just use fence_default_wait instead. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 8/8] drm/fence: add out-fences support
On Mon, Apr 25, 2016 at 07:33:28PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > Support DRM out-fences creating a sync_file with a fence for each crtc > update with the DRM_MODE_ATOMIC_OUT_FENCE flag. > > We then send an struct drm_out_fences array with the out-fences fds back in > the drm_atomic_ioctl() as an out arg in the out_fences_ptr field. > > struct drm_out_fences { > __u32 crtc_id; > __u32 fd; > }; > > v2: Comment by Rob Clark: > - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here. > > Comment by Daniel Vetter: > - Add clean up code for out_fences > > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/drm_atomic.c | 163 > +-- > include/drm/drm_crtc.h | 10 +++ > include/uapi/drm/drm_mode.h | 11 ++- > 3 files changed, 179 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 5f9d434..06c6007 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1566,6 +1566,133 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_atomic_clean_old_fb); > > +static struct drm_out_fence_state *get_out_fence(struct drm_device *dev, > + struct drm_atomic_state *state, > + uint32_t __user > *out_fences_ptr, > + uint64_t count_out_fences, > + uint64_t user_data) > +{ > + struct drm_crtc *crtc; > + struct drm_crtc_state *crtc_state; > + struct drm_out_fences *out_fences; > + struct drm_out_fence_state *fence_state; > + int num_fences = 0; > + int i, ret; > + > + if (count_out_fences > dev->mode_config.num_crtc) > + return ERR_PTR(-EINVAL); > + > + out_fences = kcalloc(count_out_fences, sizeof(*out_fences), > + GFP_KERNEL); > + if (!out_fences) > + return ERR_PTR(-ENOMEM); A bit tricky, but the above kcalloc is the only thing that catches integer overflows in count_out_fences. Needs a comment imo since this could be a security exploit if we accidentally screw it up. Also needs a testcase imo. > + > + fence_state = kcalloc(count_out_fences, sizeof(*fence_state), > + GFP_KERNEL); > + if (!fence_state) { > + kfree(out_fences); > + return ERR_PTR(-ENOMEM); > + } > + > + for (i = 0 ; i < count_out_fences ; i++) > + fence_state[i].fd = -1; > + > + for_each_crtc_in_state(state, crtc, crtc_state, i) { > + struct drm_pending_vblank_event *e; > + struct fence *fence; > + char name[32]; > + > + fence = kzalloc(sizeof(*fence), GFP_KERNEL); > + if (!fence) { > + ret = -ENOMEM; > + goto out; > + } > + > + fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, > +crtc->fence_context, crtc->fence_seqno); > + > + snprintf(name, sizeof(name), "crtc-%d_%lu", > + drm_crtc_index(crtc), crtc->fence_seqno++); Hm ... fence_init_with_name? I'm kinda confused why we only name fences that are exported though, and why not all of them. Debugging fence deadlocks is real hard, so giving them all names might be a good idea. Anyway, seems like more room for a bit more sync_file/struct fence merging. > + > + fence_state[i].fd = get_unused_fd_flags(O_CLOEXEC); > + if (fence_state[i].fd < 0) { > + fence_put(fence); > + ret = fence_state[i].fd; > + goto out; > + } > + > + fence_state[i].sync_file = sync_file_create(name, fence); > + if(!fence_state[i].sync_file) { > + fence_put(fence); > + ret = -ENOMEM; > + goto out; > + } > + > + if (crtc_state->event) { > + crtc_state->event->base.fence = fence; > + } else { This looks a bit funny - I'd change the create event logic to create an event either if we have the either drm event or out-fence flag set. > + e = create_vblank_event(dev, NULL, fence, user_data); > + if (!e) { > + ret = -ENOMEM; > + goto out; > + } > + > + crtc_state->event = e; > + } > + > + out_fences[num_fences].crtc_id = crtc->base.id; > + out_fences[num_fences].fd = fence_state[i].fd; > + num_fences++; > + } > + > + if (copy_to_user(out_fences_ptr, out_fences, > + num_fences * sizeof(*out_fences))) { > + ret = -EFAULT; > +
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
On Tue, 2016-04-26 at 11:29 -0300, Gustavo Padovan wrote: > 2016-04-26 Lucas Stach : > > Am Donnerstag, den 21.04.2016, 12:38 -0300 schrieb Gustavo Padovan: > > > From: Gustavo Padovan > > > > > > This function had copies in 3 different files. Unify them in kernel.h. > > > > > > Cc: Joe Perches > > > Cc: Andrew Morton > > > Cc: David Airlie > > > Cc: Daniel Vetter > > > Cc: Rob Clark > > > Signed-off-by: Gustavo Padovan > > > > > Though I normally prefer static inline functions, I see the benefits of > > using the macro form here. An inline could still work static inline void __user *u64_to_user_ptr(u64 address) { return (void __user *)(uintptr_t)address; } if the macro was #define u64_to_user_ptr(x) \ ({ \ typecheck(u64, x); \ (u64_to_user_ptr)(x); \ }) the parenthesis around the u64_to_user_ptr in the macro should prevent expansion. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis
Hi, Yes you are right on that regard. I did do that in my previous patches. It’s just something that I will have to remember now onwards. Thanks. Regards, Parth Sane > On 26-Apr-2016, at 8:14 PM, Luis de Bethencourt > wrote: > > On 26/04/16 15:33, Parth Sane wrote: >> Hi, >> Thanks for the feedback. I did find this issue with the assistance >> checkpatch.pl script. >> Regards, >> Parth Sane > > Normally people mention the checkpatch warning they are fixing. > > For example: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5f10ef7dce02d8e36665974aca0977cda58c1122 > > Verbosity in the commit message is a good thing :) > > I have tested that your patch still applies cleanly to Greg's staging-testing > branch. > > Thanks, > Luis > >>> On 26-Apr-2016, at 8:00 PM, Luis de Bethencourt >>> wrote: >>> >>> On 25/04/16 16:43, Parth Sane wrote: Fixed alignment to match open parenthesis. Signed-off-by: Parth Sane --- Changes in v6: -Added line before Signed-off message >>> >>> This last version looks good to me. Did you find this issue with >>> checkpatch.pl? >>> >>> Thanks, >>> Luis >>> >> signature.asc Description: Message signed with OpenPGP using GPGMail ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis
On 26/04/16 15:33, Parth Sane wrote: > Hi, > Thanks for the feedback. I did find this issue with the assistance > checkpatch.pl script. > Regards, > Parth Sane Normally people mention the checkpatch warning they are fixing. For example: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=5f10ef7dce02d8e36665974aca0977cda58c1122 Verbosity in the commit message is a good thing :) I have tested that your patch still applies cleanly to Greg's staging-testing branch. Thanks, Luis >> On 26-Apr-2016, at 8:00 PM, Luis de Bethencourt >> wrote: >> >> On 25/04/16 16:43, Parth Sane wrote: >>> Fixed alignment to match open parenthesis. >>> >>> Signed-off-by: Parth Sane >>> >>> --- >>> Changes in v6: >>> -Added line before Signed-off message >>> >> >> This last version looks good to me. Did you find this issue with >> checkpatch.pl? >> >> Thanks, >> Luis >> > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 1/8] dma-buf/fence: add fence_collection fences
On Mon, Apr 25, 2016 at 07:33:21PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > struct fence_collection inherits from struct fence and carries a > collection of fences that needs to be waited together. > > It is useful to translate a sync_file to a fence to remove the complexity > of dealing with sync_files on DRM drivers. So even if there are many > fences in the sync_file that needs to waited for a commit to happen, > they all get added to the fence_collection and passed for DRM use as > a standard struct fence. > > That means that no changes needed to any driver besides supporting fences. > > fence_collection's fence doesn't belong to any timeline context, so > fence_is_later() and fence_later() are not meant to be called with > fence_collections fences. > > v2: Comments by Daniel Vetter: > - merge fence_collection_init() and fence_collection_add() > - only add callbacks at ->enable_signalling() > - remove fence_collection_put() > - check for type on to_fence_collection() > - adjust fence_is_later() and fence_later() to WARN_ON() if they > are used with collection fences. > > Signed-off-by: Gustavo Padovan FENCE_NO_CONTEXT semantics needs an ack from amdgpu maintainers. I'm not entirely sure they might not hit the new WARN_ON by accident now. Please cc Alex Deucher & Christian König. -Daniel > --- > drivers/dma-buf/Makefile | 2 +- > drivers/dma-buf/fence-collection.c | 159 > + > drivers/dma-buf/fence.c| 2 +- > include/linux/fence-collection.h | 73 + > include/linux/fence.h | 9 +++ > 5 files changed, 243 insertions(+), 2 deletions(-) > create mode 100644 drivers/dma-buf/fence-collection.c > create mode 100644 include/linux/fence-collection.h > > diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile > index 4a424ec..52f818f 100644 > --- a/drivers/dma-buf/Makefile > +++ b/drivers/dma-buf/Makefile > @@ -1,2 +1,2 @@ > -obj-y := dma-buf.o fence.o reservation.o seqno-fence.o > +obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-collection.o > obj-$(CONFIG_SYNC_FILE) += sync_file.o > diff --git a/drivers/dma-buf/fence-collection.c > b/drivers/dma-buf/fence-collection.c > new file mode 100644 > index 000..88872e5 > --- /dev/null > +++ b/drivers/dma-buf/fence-collection.c > @@ -0,0 +1,159 @@ > +/* > + * fence-collection: aggregate fences to be waited together > + * > + * Copyright (C) 2016 Collabora Ltd > + * Authors: > + * Gustavo Padovan > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published > by > + * the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + */ > + > +#include > +#include > +#include > + > +static const char *fence_collection_get_driver_name(struct fence *fence) > +{ > + struct fence_collection *collection = to_fence_collection(fence); > + struct fence *f = collection->fences[0].fence; > + > + return f->ops->get_driver_name(fence); > +} > + > +static const char *fence_collection_get_timeline_name(struct fence *fence) > +{ > + return "no context"; > +} > + > +static void collection_check_cb_func(struct fence *fence, struct fence_cb > *cb) > +{ > + struct fence_collection_cb *f_cb; > + struct fence_collection *collection; > + > + f_cb = container_of(cb, struct fence_collection_cb, cb); > + collection = f_cb->collection; > + > + if (atomic_dec_and_test(&collection->num_pending_fences)) > + fence_signal(&collection->base); > +} > + > +static bool fence_collection_enable_signaling(struct fence *fence) > +{ > + struct fence_collection *collection = to_fence_collection(fence); > + int i; > + > + for (i = 0 ; i < collection->num_fences ; i++) { > + if (fence_add_callback(collection->fences[i].fence, > +&collection->fences[i].cb, > +collection_check_cb_func)) { > + atomic_dec(&collection->num_pending_fences); > + return false; > + } > + } > + > + return !!atomic_read(&collection->num_pending_fences); > +} > + > +static bool fence_collection_signaled(struct fence *fence) > +{ > + struct fence_collection *collection = to_fence_collection(fence); > + > + return (atomic_read(&collection->num_pending_fences) == 0); > +} > + > +static void fence_collection_release(struct fence *fence) > +{ > + struct fence_collection *collection = to_fence_collection(fence); > + int i; > + > + for (i = 0 ; i < collection->num_fences ; i++) > +
Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis
Hi, Thanks for the feedback. I did find this issue with the assistance checkpatch.pl script. Regards, Parth Sane > On 26-Apr-2016, at 8:00 PM, Luis de Bethencourt > wrote: > > On 25/04/16 16:43, Parth Sane wrote: >> Fixed alignment to match open parenthesis. >> >> Signed-off-by: Parth Sane >> >> --- >> Changes in v6: >> -Added line before Signed-off message >> > > This last version looks good to me. Did you find this issue with > checkpatch.pl? > > Thanks, > Luis > signature.asc Description: Message signed with OpenPGP using GPGMail ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis
On 25/04/16 16:43, Parth Sane wrote: > Fixed alignment to match open parenthesis. > > Signed-off-by: Parth Sane > > --- > Changes in v6: > -Added line before Signed-off message > This last version looks good to me. Did you find this issue with checkpatch.pl? Thanks, Luis ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
2016-04-26 Lucas Stach : > Am Donnerstag, den 21.04.2016, 12:38 -0300 schrieb Gustavo Padovan: > > From: Gustavo Padovan > > > > This function had copies in 3 different files. Unify them in kernel.h. > > > > Cc: Joe Perches > > Cc: Andrew Morton > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Rob Clark > > Signed-off-by: Gustavo Padovan > > > Though I normally prefer static inline functions, I see the benefits of > using the macro form here. > > For the etnaviv part: > Acked-by: Lucas Stach Thank you all! I'll collect the Acks and send a new version, so it is easier for Greg to pick it up. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: wlan-ng: memory allocated inside mkimage() is not freed if subsequent calls fails.
This patch makes the code better and it's an improvement so I'm fine with merging it as-is. On Sun, Apr 24, 2016 at 07:40:13PM +0300, Claudiu Beznea wrote: > This patch frees memory allocated inside mkimage() in case mkimage() > or any other subsequent calls inside prism2_fwapply() from prism2fw.c > file fails. To fix this I introduces goto labels where the free > operation is done in case some operations fails. After the introduction > of goto labels has been done, in order to use the same return path, > "return x" instuctions were replaced with "goto" instuctions. > > Signed-off-by: Claudiu Beznea > --- > drivers/staging/wlan-ng/prism2fw.c | 28 > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/wlan-ng/prism2fw.c > b/drivers/staging/wlan-ng/prism2fw.c > index 8564d9e..56bffd9 100644 > --- a/drivers/staging/wlan-ng/prism2fw.c > +++ b/drivers/staging/wlan-ng/prism2fw.c > @@ -278,7 +278,8 @@ static int prism2_fwapply(const struct ihex_binrec *rfptr, > /* Build the PDA we're going to use. */ > if (read_cardpda(&pda, wlandev)) { > netdev_err(wlandev->netdev, "load_cardpda failed, exiting.\n"); > - return 1; > + result = 1; > + goto out; It's better to do direct returns instead of misleading gotos that don't do anything. "Future proofing" is a waste of time and introduces more bugs than it prevents. Just write for the present. Present proof the code. > } > > /* read the card's PRI-SUP */ > @@ -315,55 +316,58 @@ static int prism2_fwapply(const struct ihex_binrec > *rfptr, > if (result) { > netdev_err(wlandev->netdev, > "Failed to read the data exiting.\n"); > - return 1; > + goto out; After this should be goto free_srecs not goto out. I don't know that it matters... > } > > result = validate_identity(); > - > if (result) { > netdev_err(wlandev->netdev, "Incompatible firmware image.\n"); > - return 1; > + goto out; > } > > if (startaddr == 0x) { > netdev_err(wlandev->netdev, > "Can't RAM download a Flash image!\n"); > - return 1; > + result = 1; > + goto out; > } > > /* Make the image chunks */ > result = mkimage(fchunk, &nfchunks); > if (result) { > netdev_err(wlandev->netdev, "Failed to make image chunk.\n"); > - return 1; > + goto free_chunks; This works, but really mkimage() should clean up it's own allocations on failure. Otherwise it is a layering violation. The rest of the patch is fine. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
研发工作的问题管理与风险管理
135791356802467913573 从技术走向管理.xls Description: Binary data ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] intel_sgx: TODO file for the staging area
On Mon, Apr 25, 2016 at 01:01:24PM -0700, Andi Kleen wrote: > Jarkko Sakkinen writes: > > > > diff --git a/drivers/staging/intel_sgx/TODO b/drivers/staging/intel_sgx/TODO > > new file mode 100644 > > index 000..05f68c2 > > --- /dev/null > > +++ b/drivers/staging/intel_sgx/TODO > > @@ -0,0 +1,25 @@ > > +Documentation > > += > > + > > +* Improve Documents/x86/intel-sgx.txt based on the feedback and > > + questions that pop up. > > + > > +Internals > > += > > + > > +* Move structures needed by the allocator to arch/x86/include/asm/sgx.h > > +* Move EPC page allocation and eviction code to arch/x86/mm as they > > + will shared with virtualization code. > > +* Move enclave management functions to arch/x86/mm as they will be > > + shared with virtualization code. > > +* Use reserve_memtype() in order to add EPC to the PAT memtype list > > + with WB caching. > > +* Implement proper recovery code for the pager for cases when > > + ETRACK/EBLOCK/EWB fails instead of BUG_ON(). Probably the sanest > > + way to recover is to clear TCS PTEs, kick threads out of enclave > > + and remove EPC pages. > > +* Implement ACPI hot-lug for SGX. > > - Write proper patch descriptions. > > Especially how the "new VM" in 3/6 works needs a lot more explanation ... Agreed. I have now idea how to improve this given the feedback so far from you Andy and Greg. Thanks. It was hard to figure out the areas, which require more explanation before putting something out first. > - Add some test code Skylake, the only microarchitecture available at the moment supporting SGX, does not support IA32_SGXLEPUBKEYHASH* MSRs documented in Volume 3C of the Intel x86 SDM. There will be an Open Source SDK available in the near future. It comes with Launch Enclave [1] that generates automatically EINITTOKENs for debug enclaves. At the moment there is no process for signing producton enclaves with the Intel root of trust for Linux (there is a process for Windows). In order to write test code I would need to use the SDK at minimum to generate EINITTOKEN for the test enclave. [1] The source code is available but with Skylake you cannot sign your own Launch Enclave binary, which is of course possible in future when the MSRs become available for having you own root of trust. > -Andi /Jarkko ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 7/8] drm/fence: add fence timeline to drm_crtc
On Mon, Apr 25, 2016 at 07:33:27PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > Create one timeline context for each CRTC to be able to handle out-fences > and signal them. It adds a few members to struct drm_crtc: fence_context, > where we store the context we get from fence_context_alloc(), the > fence seqno and the fence lock, that we pass in fence_init() to be > used by the fence. > > Signed-off-by: Gustavo Padovan > --- > drivers/gpu/drm/drm_crtc.c | 29 + > include/drm/drm_crtc.h | 19 +++ > 2 files changed, 48 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 65212ce..cf9750a 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -659,6 +659,32 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) > return num; > } > > +static const char *drm_crtc_fence_get_driver_name(struct fence *fence) > +{ > + struct drm_crtc *crtc = fence_to_crtc(fence); > + > + return crtc->dev->driver->name; > +} > + > +static const char *drm_crtc_fence_get_timeline_name(struct fence *fence) > +{ > + struct drm_crtc *crtc = fence_to_crtc(fence); > + > + return crtc->name; > +} Is that exported to userspace? crtc->name is an internal thing, not meant for outside consumption. > + > +static bool drm_crtc_fence_enable_signaling(struct fence *fence) > +{ > + return true; > +} > + > +const struct fence_ops drm_crtc_fence_ops = { > + .get_driver_name = drm_crtc_fence_get_driver_name, > + .get_timeline_name = drm_crtc_fence_get_timeline_name, > + .enable_signaling = drm_crtc_fence_enable_signaling, > + .wait = fence_default_wait, > +}; > + > /** > * drm_crtc_init_with_planes - Initialise a new CRTC object with > *specified primary and cursor planes. > @@ -709,6 +735,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, > struct drm_crtc *crtc, > return -ENOMEM; > } > > + crtc->fence_context = fence_context_alloc(1); > + spin_lock_init(&crtc->fence_lock); > + > crtc->base.properties = &crtc->properties; > > list_add_tail(&crtc->head, &config->crtc_list); > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 5ba3cda..d8c32c8 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -32,6 +32,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -715,6 +716,9 @@ struct drm_crtc_funcs { > * @helper_private: mid-layer private data > * @properties: property tracking for this CRTC > * @state: current atomic state for this CRTC > + * @fence_context: context for fence signalling > + * @fence_lock: fence lock for the fence context > + * @fence_seqno: seqno variable to create fences > * @acquire_ctx: per-CRTC implicit acquire context used by atomic drivers for > * legacy IOCTLs > * > @@ -771,6 +775,11 @@ struct drm_crtc { > > struct drm_crtc_state *state; > > + /* fence timelines info for DRM out-fences */ > + unsigned int fence_context; > + spinlock_t fence_lock; > + unsigned long fence_seqno; > + > /* >* For legacy crtc IOCTLs so that atomic drivers can get at the locking >* acquire context. > @@ -778,6 +787,16 @@ struct drm_crtc { > struct drm_modeset_acquire_ctx *acquire_ctx; > }; > > +extern const struct fence_ops drm_crtc_fence_ops; > + > +static inline struct drm_crtc *fence_to_crtc(struct fence *fence) > +{ > + if (fence->ops != &drm_crtc_fence_ops) > + return NULL; > + > + return container_of(fence->lock, struct drm_crtc, fence_lock); > +} > + > /** > * struct drm_connector_state - mutable connector state > * @connector: backpointer to the connector > -- > 2.5.5 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC v2 5/8] drm/fence: add in-fences support
On Mon, Apr 25, 2016 at 07:33:25PM -0300, Gustavo Padovan wrote: > From: Gustavo Padovan > > There is now a new property called FENCE_FD attached to every plane > state that receives the sync_file fd from userspace via the atomic commit > IOCTL. I still don't like this property abuse. Also with atomic, all passed fences must be waited upon before anything is done, so attaching them to planes seems like it might just give people the wrong idea. > > The fd is then translated to a fence (that may be a fence_collection > subclass or just a normal fence) and then used by DRM to fence_wait() for > all fences in the sync_file to signal. So it only commits when all > framebuffers are ready to scanout. > > Signed-off-by: Gustavo Padovan > > v2: Comments by Daniel Vetter: > - remove set state->fence = NULL in destroy phase > - accept fence -1 as valid and just return 0 > - do not call fence_get() - sync_file_fences_get() already calls it > - fence_put() if state->fence is already set, in case userspace > set the property more than once. > --- > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/drm_atomic.c| 14 ++ > drivers/gpu/drm/drm_atomic_helper.c | 3 +++ > drivers/gpu/drm/drm_crtc.c | 7 +++ > include/drm/drm_crtc.h | 1 + > 5 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index f2a74d0..3c987e3 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -12,6 +12,7 @@ menuconfig DRM > select I2C > select I2C_ALGOBIT > select DMA_SHARED_BUFFER > + select SYNC_FILE > help > Kernel-level support for the Direct Rendering Infrastructure (DRI) > introduced in XFree86 4.0. If you say Y here, you need to select > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8ee1db8..13674c7 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -30,6 +30,7 @@ > #include > #include > #include > +#include > > /** > * drm_atomic_state_default_release - > @@ -680,6 +681,17 @@ int drm_atomic_plane_set_property(struct drm_plane > *plane, > drm_atomic_set_fb_for_plane(state, fb); > if (fb) > drm_framebuffer_unreference(fb); > + } else if (property == config->prop_fence_fd) { > + if (U642I64(val) == -1) > + return 0; > + > + if (state->fence) > + fence_put(state->fence); > + > + state->fence = sync_file_fences_get(val); > + if (!state->fence) > + return -EINVAL; > + > } else if (property == config->prop_crtc_id) { > struct drm_crtc *crtc = drm_crtc_find(dev, val); > return drm_atomic_set_crtc_for_plane(state, crtc); > @@ -737,6 +749,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane, > > if (property == config->prop_fb_id) { > *val = (state->fb) ? state->fb->base.id : 0; > + } else if (property == config->prop_fence_fd) { > + *val = -1; > } else if (property == config->prop_crtc_id) { > *val = (state->crtc) ? state->crtc->base.id : 0; > } else if (property == config->prop_crtc_x) { > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index f1cfcce..866f332 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2696,6 +2696,9 @@ void __drm_atomic_helper_plane_destroy_state(struct > drm_plane *plane, > { > if (state->fb) > drm_framebuffer_unreference(state->fb); > + > + if (state->fence) > + fence_put(state->fence); > } > EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state); > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 55ffde5..65212ce 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1278,6 +1278,7 @@ int drm_universal_plane_init(struct drm_device *dev, > struct drm_plane *plane, > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { > drm_object_attach_property(&plane->base, config->prop_fb_id, 0); > + drm_object_attach_property(&plane->base, config->prop_fence_fd, > -1); > drm_object_attach_property(&plane->base, config->prop_crtc_id, > 0); > drm_object_attach_property(&plane->base, config->prop_crtc_x, > 0); > drm_object_attach_property(&plane->base, config->prop_crtc_y, > 0); > @@ -1533,6 +1534,12 @@ static int drm_mode_create_standard_properties(struct > drm_device *dev) > return -ENOMEM; > dev->mode_config.prop_fb_id = prop; > > + prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC, > + "FENCE_FD", -1, INT_MAX); > + if (!prop) > + ret
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
Am Donnerstag, den 21.04.2016, 12:38 -0300 schrieb Gustavo Padovan: > From: Gustavo Padovan > > This function had copies in 3 different files. Unify them in kernel.h. > > Cc: Joe Perches > Cc: Andrew Morton > Cc: David Airlie > Cc: Daniel Vetter > Cc: Rob Clark > Signed-off-by: Gustavo Padovan > Though I normally prefer static inline functions, I see the benefits of using the macro form here. For the etnaviv part: Acked-by: Lucas Stach ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel