Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
On 2021-03-11 08:26, Christoph Hellwig wrote: On Wed, Mar 10, 2021 at 06:39:57PM +, Robin Murphy wrote: Actually... Just mirroring the iommu_dma_strict value into struct iommu_domain should solve all of that with very little boilerplate code. Yes, my initial thought was to directly replace the attribute with a common flag at iommu_domain level, but since in all cases the behaviour is effectively global rather than actually per-domain, it seemed reasonable to take it a step further. This passes compile-testing for arm64 and x86, what do you think? It seems to miss a few bits, and also generally seems to be not actually apply to recent mainline or something like it due to different empty lines in a few places. Yeah, that was sketched out on top of some other development patches, and in being so focused on not breaking any of the x86 behaviours I did indeed overlook fully converting the SMMU drivers... oops! (my thought was to do the conversion for its own sake, then clean up the redundant attribute separately, but I guess it's fine either way) Let me know what you think of the version here: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/iommu-cleanup I'll happily switch the patch to you as the author if you're fine with that as well. I still have reservations about removing the attribute API entirely and pretending that io_pgtable_cfg is anything other than a SoC-specific private interface, but the reworked patch on its own looks reasonable to me, thanks! (I wasn't too convinced about the iommu_cmd_line wrappers either...) Just iommu_get_dma_strict() needs an export since the SMMU drivers can be modular - I consciously didn't add that myself since I was mistakenly thinking only iommu-dma would call it. Robin. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 10/22] virtio/vsock: simplify credit update function API
On Sun, Mar 07, 2021 at 09:01:44PM +0300, Arseny Krasnov wrote: This function is static and 'hdr' arg was always NULL. Signed-off-by: Arseny Krasnov --- net/vmw_vsock/virtio_transport_common.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 09/22] virtio/vsock: set packet's type in virtio_transport_send_pkt_info()
On Sun, Mar 07, 2021 at 09:01:22PM +0300, Arseny Krasnov wrote: This moves passing type of packet from 'info' structure to 'virtio_ transport_send_pkt_info()' function. There is no need to set type of packet which differs from type of socket. Since at current time only stream type is supported, set it directly in 'virtio_transport_send_ pkt_info()', so callers don't need to set it. Signed-off-by: Arseny Krasnov --- net/vmw_vsock/virtio_transport_common.c | 19 +-- 1 file changed, 5 insertions(+), 14 deletions(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 08/22] af_vsock: update comments for stream sockets
On Sun, Mar 07, 2021 at 09:01:05PM +0300, Arseny Krasnov wrote: This replaces 'stream' to 'connection oriented' in comments as SEQPACKET is also connection oriented. Signed-off-by: Arseny Krasnov --- net/vmw_vsock/af_vsock.c | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 07/22] af_vsock: rest of SEQPACKET support
On Sun, Mar 07, 2021 at 09:00:47PM +0300, Arseny Krasnov wrote: This does rest of SOCK_SEQPACKET support: 1) Adds socket ops for SEQPACKET type. 2) Allows to create socket with SEQPACKET type. Signed-off-by: Arseny Krasnov --- include/net/af_vsock.h | 1 + net/vmw_vsock/af_vsock.c | 36 +++- 2 files changed, 36 insertions(+), 1 deletion(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 12/22] virtio/vsock: fetch length for SEQPACKET record
On Sun, Mar 07, 2021 at 09:02:31PM +0300, Arseny Krasnov wrote:
This adds transport callback which tries to fetch record begin marker
from socket's rx queue. It is called from af_vsock.c before reading data
packets of record.
Signed-off-by: Arseny Krasnov
---
include/linux/virtio_vsock.h| 1 +
net/vmw_vsock/virtio_transport_common.c | 53 +
2 files changed, 54 insertions(+)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 466a5832d2f5..d7edcfeb4cd2 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -88,6 +88,7 @@ virtio_transport_dgram_dequeue(struct vsock_sock *vsk,
struct msghdr *msg,
size_t len, int flags);
+size_t virtio_transport_seqpacket_seq_get_len(struct vsock_sock *vsk);
int
virtio_transport_seqpacket_dequeue(struct vsock_sock *vsk,
struct msghdr *msg,
diff --git a/net/vmw_vsock/virtio_transport_common.c
b/net/vmw_vsock/virtio_transport_common.c
index 5f1e283e43f3..6fc78fec41c0 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -399,6 +399,59 @@ static inline void virtio_transport_remove_pkt(struct
virtio_vsock_pkt *pkt)
virtio_transport_free_pkt(pkt);
}
+static size_t virtio_transport_drop_until_seq_begin(struct virtio_vsock_sock
*vvs)
+{
+ struct virtio_vsock_pkt *pkt, *n;
+ size_t bytes_dropped = 0;
+
+ list_for_each_entry_safe(pkt, n, >rx_queue, list) {
+ if (le16_to_cpu(pkt->hdr.op) == VIRTIO_VSOCK_OP_SEQ_BEGIN)
+ break;
+
+ bytes_dropped += le32_to_cpu(pkt->hdr.len);
+ virtio_transport_dec_rx_pkt(vvs, pkt);
+ virtio_transport_remove_pkt(pkt);
+ }
+
+ return bytes_dropped;
+}
+
+size_t virtio_transport_seqpacket_seq_get_len(struct vsock_sock *vsk)
+{
+ struct virtio_vsock_seq_hdr *seq_hdr;
+ struct virtio_vsock_sock *vvs;
+ struct virtio_vsock_pkt *pkt;
+ size_t bytes_dropped;
+
+ vvs = vsk->trans;
+
+ spin_lock_bh(>rx_lock);
+
+ /* Fetch all orphaned 'RW' packets and send credit update. */
+ bytes_dropped = virtio_transport_drop_until_seq_begin(vvs);
+
+ if (list_empty(>rx_queue))
+ goto out;
What do we return to in this case?
IIUC we return the len of the previous packet, should we set
vvs->seqpacket_state.user_read_seq_len to 0?
+
+ pkt = list_first_entry(>rx_queue, struct virtio_vsock_pkt, list);
+
+ vvs->seqpacket_state.user_read_copied = 0;
+
+ seq_hdr = (struct virtio_vsock_seq_hdr *)pkt->buf;
+ vvs->seqpacket_state.user_read_seq_len =
le32_to_cpu(seq_hdr->msg_len);
+ vvs->seqpacket_state.curr_rx_msg_id = le32_to_cpu(seq_hdr->msg_id);
+ virtio_transport_dec_rx_pkt(vvs, pkt);
+ virtio_transport_remove_pkt(pkt);
+out:
+ spin_unlock_bh(>rx_lock);
+
+ if (bytes_dropped)
+ virtio_transport_send_credit_update(vsk);
+
+ return vvs->seqpacket_state.user_read_seq_len;
+}
+EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_seq_get_len);
+
static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
struct msghdr *msg,
bool *msg_ready)
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 04/22] af_vsock: implement SEQPACKET receive loop
On Sun, Mar 07, 2021 at 08:59:45PM +0300, Arseny Krasnov wrote:
This adds receive loop for SEQPACKET. It looks like receive loop for
STREAM, but there is a little bit difference:
1) It doesn't call notify callbacks.
2) It doesn't care about 'SO_SNDLOWAT' and 'SO_RCVLOWAT' values, because
there is no sense for these values in SEQPACKET case.
3) It waits until whole record is received or error is found during
receiving.
4) It processes and sets 'MSG_TRUNC' flag.
So to avoid extra conditions for two types of socket inside one loop, two
independent functions were created.
Signed-off-by: Arseny Krasnov
---
include/net/af_vsock.h | 5 +++
net/vmw_vsock/af_vsock.c | 95 +++-
2 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index b1c717286993..5ad7ee7f78fd 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -135,6 +135,11 @@ struct vsock_transport {
bool (*stream_is_active)(struct vsock_sock *);
bool (*stream_allow)(u32 cid, u32 port);
+ /* SEQ_PACKET. */
+ size_t (*seqpacket_seq_get_len)(struct vsock_sock *vsk);
+ int (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
+int flags, bool *msg_ready);
+
/* Notification. */
int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);
int (*notify_poll_out)(struct vsock_sock *, size_t, bool *);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 0bc661e54262..ac2f69362f2e 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1973,6 +1973,96 @@ static int __vsock_stream_recvmsg(struct sock *sk,
struct msghdr *msg,
return err;
}
+static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
+size_t len, int flags)
+{
+ const struct vsock_transport *transport;
+ const struct iovec *orig_iov;
+ unsigned long orig_nr_segs;
+ bool msg_ready;
+ struct vsock_sock *vsk;
+ size_t record_len;
+ long timeout;
+ int err = 0;
+ DEFINE_WAIT(wait);
+
+ vsk = vsock_sk(sk);
+ transport = vsk->transport;
+
+ timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+ orig_nr_segs = msg->msg_iter.nr_segs;
+ orig_iov = msg->msg_iter.iov;
+ msg_ready = false;
+ record_len = 0;
+
+ while (1) {
+ err = vsock_wait_data(sk, , timeout, NULL, 0);
+
+ if (err <= 0) {
+ /* In case of any loop break(timeout, signal
+* interrupt or shutdown), we report user that
+* nothing was copied.
+*/
+ err = 0;
+ break;
+ }
+
+ if (record_len == 0) {
+ record_len =
+ transport->seqpacket_seq_get_len(vsk);
+
+ if (record_len == 0)
+ continue;
+ }
+
+ err = transport->seqpacket_dequeue(vsk, msg, flags, _ready);
In order to simplify the transport interface, can we do the work of
seqpacket_seq_get_len() at the beginning of seqpacket_dequeue()?
So in this way seqpacket_dequeue() can return the 'record_len' or an
error.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 06/22] af_vsock: implement send logic for SEQPACKET
On Sun, Mar 07, 2021 at 09:00:26PM +0300, Arseny Krasnov wrote:
This adds some logic to current stream enqueue function for SEQPACKET
support:
1) Use transport's seqpacket enqueue callback.
2) Return value from enqueue function is whole record length or error
for SOCK_SEQPACKET.
Signed-off-by: Arseny Krasnov
---
include/net/af_vsock.h | 2 ++
net/vmw_vsock/af_vsock.c | 22 --
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index a8c4039e40cf..aed306292ab3 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -139,6 +139,8 @@ struct vsock_transport {
size_t (*seqpacket_seq_get_len)(struct vsock_sock *vsk);
int (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
int flags, bool *msg_ready);
+ int (*seqpacket_enqueue)(struct vsock_sock *vsk, struct msghdr *msg,
+int flags, size_t len);
/* Notification. */
int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 5bf64a3e678a..a031f165494d 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1830,9 +1830,14 @@ static int vsock_connectible_sendmsg(struct socket
*sock, struct msghdr *msg,
* responsibility to check how many bytes we were able to send.
*/
- written = transport->stream_enqueue(
- vsk, msg,
- len - total_written);
+ if (sk->sk_type == SOCK_SEQPACKET) {
+ written = transport->seqpacket_enqueue(vsk,
+ msg, msg->msg_flags,
I think we can avoid to pass 'msg->msg_flags', since the transport can
access it through the 'msg' pointer, right?
+ len - total_written);
+ } else {
+ written = transport->stream_enqueue(vsk,
+ msg, len - total_written);
+ }
if (written < 0) {
err = -ENOMEM;
goto out_err;
@@ -1844,12 +1849,17 @@ static int vsock_connectible_sendmsg(struct socket
*sock, struct msghdr *msg,
vsk, written, _data);
if (err < 0)
goto out_err;
-
}
out_err:
- if (total_written > 0)
- err = total_written;
+ if (total_written > 0) {
+ /* Return number of written bytes only if:
+* 1) SOCK_STREAM socket.
+* 2) SOCK_SEQPACKET socket when whole buffer is sent.
+*/
+ if (sk->sk_type == SOCK_STREAM || total_written == len)
+ err = total_written;
+ }
out:
release_sock(sk);
return err;
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 04/22] af_vsock: implement SEQPACKET receive loop
On Sun, Mar 07, 2021 at 08:59:45PM +0300, Arseny Krasnov wrote: This adds receive loop for SEQPACKET. It looks like receive loop for STREAM, but there is a little bit difference: 1) It doesn't call notify callbacks. 2) It doesn't care about 'SO_SNDLOWAT' and 'SO_RCVLOWAT' values, because there is no sense for these values in SEQPACKET case. 3) It waits until whole record is received or error is found during receiving. 4) It processes and sets 'MSG_TRUNC' flag. So to avoid extra conditions for two types of socket inside one loop, two independent functions were created. Signed-off-by: Arseny Krasnov --- include/net/af_vsock.h | 5 +++ net/vmw_vsock/af_vsock.c | 95 +++- 2 files changed, 99 insertions(+), 1 deletion(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 02/22] af_vsock: separate wait data loop
On Sun, Mar 07, 2021 at 08:59:01PM +0300, Arseny Krasnov wrote: This moves wait loop for data to dedicated function, because later it will be used by SEQPACKET data receive loop. While moving the code around, let's update an old comment. Signed-off-by: Arseny Krasnov --- net/vmw_vsock/af_vsock.c | 156 +-- 1 file changed, 84 insertions(+), 72 deletions(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH v6 01/22] af_vsock: update functions for connectible socket
On Sun, Mar 07, 2021 at 08:58:39PM +0300, Arseny Krasnov wrote: This prepares af_vsock.c for SEQPACKET support: some functions such as setsockopt(), getsockopt(), connect(), recvmsg(), sendmsg() are shared between both types of sockets, so rename them in general manner. Signed-off-by: Arseny Krasnov --- net/vmw_vsock/af_vsock.c | 64 +--- 1 file changed, 34 insertions(+), 30 deletions(-) Reviewed-by: Stefano Garzarella ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vhost: Fix vhost_vq_reset()
vhost_reset_is_le() is vhost_init_is_le(), and in the case of cross-endian legacy, vhost_init_is_le() depends on vq->user_be. vq->user_be is set by vhost_disable_cross_endian(). But in vhost_vq_reset(), we have: vhost_reset_is_le(vq); vhost_disable_cross_endian(vq); And so user_be is used before being set. To fix that, reverse the lines order as there is no other dependency between them. Signed-off-by: Laurent Vivier --- drivers/vhost/vhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index a262e12c6dc2..5ccb0705beae 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -332,8 +332,8 @@ static void vhost_vq_reset(struct vhost_dev *dev, vq->error_ctx = NULL; vq->kick = NULL; vq->log_ctx = NULL; - vhost_reset_is_le(vq); vhost_disable_cross_endian(vq); + vhost_reset_is_le(vq); vq->busyloop_timeout = 0; vq->umem = NULL; vq->iotlb = NULL; -- 2.29.2 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 7/8] x86/boot/compressed/64: Check SEV encryption in 32-bit boot-path
From: Joerg Roedel Check whether the hypervisor reported the correct C-bit when running as an SEV guest. Using a wrong C-bit position could be used to leak sensitive data from the guest to the hypervisor. Signed-off-by: Joerg Roedel --- arch/x86/boot/compressed/head_64.S | 83 ++ 1 file changed, 83 insertions(+) diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index ee448aedb8b0..7c5c2698a96e 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -183,11 +183,21 @@ SYM_FUNC_START(startup_32) */ callget_sev_encryption_bit xorl%edx, %edx +#ifdef CONFIG_AMD_MEM_ENCRYPT testl %eax, %eax jz 1f subl$32, %eax /* Encryption bit is always above bit 31 */ bts %eax, %edx /* Set encryption mask for page tables */ + /* +* Mark SEV as active in sev_status so that startup32_check_sev_cbit() +* will do a check. The sev_status memory will be fully initialized +* with the contents of MSR_AMD_SEV_STATUS later in +* set_sev_encryption_mask(). For now it is sufficient to know that SEV +* is active. +*/ + movl$1, rva(sev_status)(%ebp) 1: +#endif /* Initialize Page tables to 0 */ lealrva(pgtable)(%ebx), %edi @@ -272,6 +282,9 @@ SYM_FUNC_START(startup_32) movl%esi, %edx 1: #endif + /* Check if the C-bit position is correct when SEV is active */ + callstartup32_check_sev_cbit + pushl $__KERNEL_CS pushl %eax @@ -871,6 +884,76 @@ SYM_FUNC_START(startup32_load_idt) ret SYM_FUNC_END(startup32_load_idt) +/* + * Check for the correct C-bit position when the startup_32 boot-path is used. + * + * The check makes use of the fact that all memory is encrypted when paging is + * disabled. The function creates 64 bits of random data using the RDRAND + * instruction. RDRAND is mandatory for SEV guests, so always available. If the + * hypervisor violates that the kernel will crash right here. + * + * The 64 bits of random data are stored to a memory location and at the same + * time kept in the %eax and %ebx registers. Since encryption is always active + * when paging is off the random data will be stored encrypted in main memory. + * + * Then paging is enabled. When the C-bit position is correct all memory is + * still mapped encrypted and comparing the register values with memory will + * succeed. An incorrect C-bit position will map all memory unencrypted, so that + * the compare will use the encrypted random data and fail. + */ +SYM_FUNC_START(startup32_check_sev_cbit) +#ifdef CONFIG_AMD_MEM_ENCRYPT + pushl %eax + pushl %ebx + pushl %ecx + pushl %edx + + /* Check for non-zero sev_status */ + movlrva(sev_status)(%ebp), %eax + testl %eax, %eax + jz 4f + + /* +* Get two 32-bit random values - Don't bail out if RDRAND fails +* because it is better to prevent forward progress if no random value +* can be gathered. +*/ +1: rdrand %eax + jnc 1b +2: rdrand %ebx + jnc 2b + + /* Store to memory and keep it in the registers */ + movl%eax, rva(sev_check_data)(%ebp) + movl%ebx, rva(sev_check_data+4)(%ebp) + + /* Enable paging to see if encryption is active */ + movl%cr0, %edx /* Backup %cr0 in %edx */ + movl$(X86_CR0_PG | X86_CR0_PE), %ecx /* Enable Paging and Protected mode */ + movl%ecx, %cr0 + + cmpl%eax, rva(sev_check_data)(%ebp) + jne 3f + cmpl%ebx, rva(sev_check_data+4)(%ebp) + jne 3f + + movl%edx, %cr0 /* Restore previous %cr0 */ + + jmp 4f + +3: /* Check failed - hlt the machine */ + hlt + jmp 3b + +4: + popl%edx + popl%ecx + popl%ebx + popl%eax +#endif + ret +SYM_FUNC_END(startup32_check_sev_cbit) + /* * Stack and heap for uncompression */ -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 5/8] x86/boot/compressed/64: Add 32-bit boot #VC handler
From: Joerg Roedel Add a #VC exception handler which is used when the kernel still executes in protected mode. This boot-path already uses CPUID, which will cause #VC exceptions in an SEV-ES guest. Signed-off-by: Joerg Roedel --- arch/x86/boot/compressed/head_64.S | 6 ++ arch/x86/boot/compressed/mem_encrypt.S | 96 +- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index 2001c3bf0748..ee448aedb8b0 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -34,6 +34,7 @@ #include #include #include +#include #include "pgtable.h" /* @@ -857,6 +858,11 @@ SYM_FUNC_END(startup32_set_idt_entry) SYM_FUNC_START(startup32_load_idt) #ifdef CONFIG_AMD_MEM_ENCRYPT + /* #VC handler */ + lealrva(startup32_vc_handler)(%ebp), %eax + movl$X86_TRAP_VC, %edx + callstartup32_set_idt_entry + /* Load IDT */ lealrva(boot32_idt)(%ebp), %eax movl%eax, rva(boot32_idt_desc+2)(%ebp) diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S index a6dea4e8a082..e915f4906477 100644 --- a/arch/x86/boot/compressed/mem_encrypt.S +++ b/arch/x86/boot/compressed/mem_encrypt.S @@ -61,10 +61,104 @@ SYM_FUNC_START(get_sev_encryption_bit) ret SYM_FUNC_END(get_sev_encryption_bit) +/** + * sev_es_req_cpuid - Request a CPUID value from the Hypervisor using + * the GHCB MSR protocol + * + * @%eax: Register to request (0=EAX, 1=EBX, 2=ECX, 3=EDX) + * @%edx: CPUID Function + * + * Returns 0 in %eax on sucess, non-zero on failure + * %edx returns CPUID value on success + */ +SYM_CODE_START_LOCAL(sev_es_req_cpuid) + shll$30, %eax + orl $0x0004, %eax + movl$MSR_AMD64_SEV_ES_GHCB, %ecx + wrmsr + rep; vmmcall# VMGEXIT + rdmsr + + /* Check response */ + movl%eax, %ecx + andl$0x3000, %ecx # Bits [12-29] MBZ + jnz 2f + + /* Check return code */ + andl$0xfff, %eax + cmpl$5, %eax + jne 2f + + /* All good - return success */ + xorl%eax, %eax +1: + ret +2: + movl$-1, %eax + jmp 1b +SYM_CODE_END(sev_es_req_cpuid) + +SYM_CODE_START(startup32_vc_handler) + pushl %eax + pushl %ebx + pushl %ecx + pushl %edx + + /* Keep CPUID function in %ebx */ + movl%eax, %ebx + + /* Check if error-code == SVM_EXIT_CPUID */ + cmpl$0x72, 16(%esp) + jne .Lfail + + movl$0, %eax# Request CPUID[fn].EAX + movl%ebx, %edx # CPUID fn + callsev_es_req_cpuid# Call helper + testl %eax, %eax # Check return code + jnz .Lfail + movl%edx, 12(%esp) # Store result + + movl$1, %eax# Request CPUID[fn].EBX + movl%ebx, %edx # CPUID fn + callsev_es_req_cpuid# Call helper + testl %eax, %eax # Check return code + jnz .Lfail + movl%edx, 8(%esp) # Store result + + movl$2, %eax# Request CPUID[fn].ECX + movl%ebx, %edx # CPUID fn + callsev_es_req_cpuid# Call helper + testl %eax, %eax # Check return code + jnz .Lfail + movl%edx, 4(%esp) # Store result + + movl$3, %eax# Request CPUID[fn].EDX + movl%ebx, %edx # CPUID fn + callsev_es_req_cpuid# Call helper + testl %eax, %eax # Check return code + jnz .Lfail + movl%edx, 0(%esp) # Store result + + popl%edx + popl%ecx + popl%ebx + popl%eax + + /* Remove error code */ + addl$4, %esp + + /* Jump over CPUID instruction */ + addl$2, (%esp) + + iret +.Lfail: + hlt + jmp .Lfail +SYM_CODE_END(startup32_vc_handler) + .code64 #include "../../kernel/sev_verify_cbit.S" - SYM_FUNC_START(set_sev_encryption_mask) #ifdef CONFIG_AMD_MEM_ENCRYPT push%rbp -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 8/8] x86/sev-es: Replace open-coded hlt-loops with sev_es_terminate()
From: Joerg Roedel
There are a few places left in the SEV-ES C code where hlt loops and/or
terminate requests are implemented. Replace them all with calls to
sev_es_terminate().
Signed-off-by: Joerg Roedel
---
arch/x86/boot/compressed/sev-es.c | 12 +++-
arch/x86/kernel/sev-es-shared.c | 10 +++---
2 files changed, 6 insertions(+), 16 deletions(-)
diff --git a/arch/x86/boot/compressed/sev-es.c
b/arch/x86/boot/compressed/sev-es.c
index 27826c265aab..d904bd56b3e3 100644
--- a/arch/x86/boot/compressed/sev-es.c
+++ b/arch/x86/boot/compressed/sev-es.c
@@ -200,14 +200,8 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long
exit_code)
}
finish:
- if (result == ES_OK) {
+ if (result == ES_OK)
vc_finish_insn();
- } else if (result != ES_RETRY) {
- /*
-* For now, just halt the machine. That makes debugging easier,
-* later we just call sev_es_terminate() here.
-*/
- while (true)
- asm volatile("hlt\n");
- }
+ else if (result != ES_RETRY)
+ sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST);
}
diff --git a/arch/x86/kernel/sev-es-shared.c b/arch/x86/kernel/sev-es-shared.c
index 387b71669818..0aa9f13efd57 100644
--- a/arch/x86/kernel/sev-es-shared.c
+++ b/arch/x86/kernel/sev-es-shared.c
@@ -24,7 +24,7 @@ static bool __init sev_es_check_cpu_features(void)
return true;
}
-static void sev_es_terminate(unsigned int reason)
+static void __noreturn sev_es_terminate(unsigned int reason)
{
u64 val = GHCB_SEV_TERMINATE;
@@ -206,12 +206,8 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned
long exit_code)
return;
fail:
- sev_es_wr_ghcb_msr(GHCB_SEV_TERMINATE);
- VMGEXIT();
-
- /* Shouldn't get here - if we do halt the machine */
- while (true)
- asm volatile("hlt\n");
+ /* Terminate the guest */
+ sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST);
}
static enum es_result vc_insn_string_read(struct es_em_ctxt *ctxt,
--
2.30.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 4/8] x86/boot/compressed/64: Setup IDT in startup_32 boot path
From: Joerg Roedel This boot path needs exception handling when it is used with SEV-ES. Setup an IDT and provide a helper function to write IDT entries for use in 32-bit protected mode. Signed-off-by: Joerg Roedel --- arch/x86/boot/compressed/head_64.S | 72 ++ 1 file changed, 72 insertions(+) diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index c59c80ca546d..2001c3bf0748 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -116,6 +116,9 @@ SYM_FUNC_START(startup_32) lretl 1: + /* Setup Exception handling for SEV-ES */ + callstartup32_load_idt + /* Make sure cpu supports long mode. */ callverify_cpu testl %eax, %eax @@ -701,6 +704,19 @@ SYM_DATA_START(boot_idt) .endr SYM_DATA_END_LABEL(boot_idt, SYM_L_GLOBAL, boot_idt_end) +#ifdef CONFIG_AMD_MEM_ENCRYPT +SYM_DATA_START(boot32_idt_desc) + .word boot32_idt_end - boot32_idt - 1 + .long 0 +SYM_DATA_END(boot32_idt_desc) + .balign 8 +SYM_DATA_START(boot32_idt) + .rept 32 + .quad 0 + .endr +SYM_DATA_END_LABEL(boot32_idt, SYM_L_GLOBAL, boot32_idt_end) +#endif + #ifdef CONFIG_EFI_STUB SYM_DATA(image_offset, .long 0) #endif @@ -793,6 +809,62 @@ SYM_DATA_START_LOCAL(loaded_image_proto) SYM_DATA_END(loaded_image_proto) #endif +#ifdef CONFIG_AMD_MEM_ENCRYPT + __HEAD + .code32 +/* + * Write an IDT entry into boot32_idt + * + * Parameters: + * + * %eax: Handler address + * %edx: Vector number + * + * Physical offset is expected in %ebp + */ +SYM_FUNC_START(startup32_set_idt_entry) + push%ebx + push%ecx + + /* IDT entry address to %ebx */ + lealrva(boot32_idt)(%ebp), %ebx + shl $3, %edx + addl%edx, %ebx + + /* Build IDT entry, lower 4 bytes */ + movl%eax, %edx + andl$0x, %edx # Target code segment offset [15:0] + movl$__KERNEL32_CS, %ecx# Target code segment selector + shl $16, %ecx + orl %ecx, %edx + + /* Store lower 4 bytes to IDT */ + movl%edx, (%ebx) + + /* Build IDT entry, upper 4 bytes */ + movl%eax, %edx + andl$0x, %edx # Target code segment offset [31:16] + orl $0x8e00, %edx # Present, Type 32-bit Interrupt Gate + + /* Store upper 4 bytes to IDT */ + movl%edx, 4(%ebx) + + pop %ecx + pop %ebx + ret +SYM_FUNC_END(startup32_set_idt_entry) +#endif + +SYM_FUNC_START(startup32_load_idt) +#ifdef CONFIG_AMD_MEM_ENCRYPT + /* Load IDT */ + lealrva(boot32_idt)(%ebp), %eax + movl%eax, rva(boot32_idt_desc+2)(%ebp) + lidtrva(boot32_idt_desc)(%ebp) +#endif + ret +SYM_FUNC_END(startup32_load_idt) + /* * Stack and heap for uncompression */ -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 2/8] x86/sev: Do not require Hypervisor CPUID bit for SEV guests
From: Joerg Roedel
A malicious hypervisor could disable the CPUID intercept for an SEV or
SEV-ES guest and trick it into the no-SEV boot path, where it could
potentially reveal secrets. This is not an issue for SEV-SNP guests,
as the CPUID intercept can't be disabled for those.
Remove the Hypervisor CPUID bit check from the SEV detection code to
protect against this kind of attack and add a Hypervisor bit equals
zero check to the SME detection path to prevent non-SEV guests from
trying to enable SME.
This handles the following cases:
1) SEV(-ES) guest where CPUID intercept is disabled. The guest
will still see leaf 0x801f and the SEV bit. It can
retrieve the C-bit and boot normally.
2) Non-SEV guests with intercepted CPUID will check SEV_STATUS
MSR and find it 0 and will try to enable SME. This will
fail when the guest finds MSR_K8_SYSCFG to be zero, as it
is emulated by KVM. But we can't rely on that, as there
might be other hypervisors which return this MSR with bit
23 set. The Hypervisor bit check will prevent that the
guest tries to enable SME in this case.
3) Non-SEV guests on SEV capable hosts with CPUID intercept
disabled (by a malicious hypervisor) will try to boot into
the SME path. This will fail, but it is also not considered
a problem because non-encrypted guests have no protection
against the hypervisor anyway.
Signed-off-by: Joerg Roedel
---
arch/x86/boot/compressed/mem_encrypt.S | 6 -
arch/x86/kernel/sev-es-shared.c| 6 +
arch/x86/mm/mem_encrypt_identity.c | 35 ++
3 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/arch/x86/boot/compressed/mem_encrypt.S
b/arch/x86/boot/compressed/mem_encrypt.S
index aa561795efd1..a6dea4e8a082 100644
--- a/arch/x86/boot/compressed/mem_encrypt.S
+++ b/arch/x86/boot/compressed/mem_encrypt.S
@@ -23,12 +23,6 @@ SYM_FUNC_START(get_sev_encryption_bit)
push%ecx
push%edx
- /* Check if running under a hypervisor */
- movl$1, %eax
- cpuid
- bt $31, %ecx /* Check the hypervisor bit */
- jnc .Lno_sev
-
movl$0x8000, %eax /* CPUID to check the highest leaf */
cpuid
cmpl$0x801f, %eax /* See if 0x801f is available */
diff --git a/arch/x86/kernel/sev-es-shared.c b/arch/x86/kernel/sev-es-shared.c
index cdc04d091242..387b71669818 100644
--- a/arch/x86/kernel/sev-es-shared.c
+++ b/arch/x86/kernel/sev-es-shared.c
@@ -186,7 +186,6 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned
long exit_code)
* make it accessible to the hypervisor.
*
* In particular, check for:
-* - Hypervisor CPUID bit
* - Availability of CPUID leaf 0x801f
* - SEV CPUID bit.
*
@@ -194,10 +193,7 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned
long exit_code)
* can't be checked here.
*/
- if ((fn == 1 && !(regs->cx & BIT(31
- /* Hypervisor bit */
- goto fail;
- else if (fn == 0x8000 && (regs->ax < 0x801f))
+ if (fn == 0x8000 && (regs->ax < 0x801f))
/* SEV leaf check */
goto fail;
else if ((fn == 0x801f && !(regs->ax & BIT(1
diff --git a/arch/x86/mm/mem_encrypt_identity.c
b/arch/x86/mm/mem_encrypt_identity.c
index 6c5eb6f3f14f..a19374d26101 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -503,14 +503,10 @@ void __init sme_enable(struct boot_params *bp)
#define AMD_SME_BITBIT(0)
#define AMD_SEV_BITBIT(1)
- /*
-* Set the feature mask (SME or SEV) based on whether we are
-* running under a hypervisor.
-*/
- eax = 1;
- ecx = 0;
- native_cpuid(, , , );
- feature_mask = (ecx & BIT(31)) ? AMD_SEV_BIT : AMD_SME_BIT;
+
+ /* Check the SEV MSR whether SEV or SME is enabled */
+ sev_status = __rdmsr(MSR_AMD64_SEV);
+ feature_mask = (sev_status & MSR_AMD64_SEV_ENABLED) ? AMD_SEV_BIT :
AMD_SME_BIT;
/*
* Check for the SME/SEV feature:
@@ -530,19 +526,26 @@ void __init sme_enable(struct boot_params *bp)
/* Check if memory encryption is enabled */
if (feature_mask == AMD_SME_BIT) {
+ /*
+* No SME if Hypervisor bit is set. This check is here to
+* prevent a guest from trying to enable SME. For running as a
+* KVM guest the MSR_K8_SYSCFG will be sufficient, but there
+* might be other hypervisors which emulate that MSR as non-zero
+* or even pass it through to the guest.
+* A malicious hypervisor can still trick a guest into this
+* path, but
[PATCH v3 6/8] x86/boot/compressed/64: Add CPUID sanity check to 32-bit boot-path
From: Joerg Roedel The 32-bit #VC handler has no GHCB and can only handle CPUID exit codes. It is needed by the early boot code to handle #VC exceptions raised in verify_cpu() and to get the position of the C bit. But the CPUID information comes from the hypervisor, which is untrusted and might return results which trick the guest into the no-SEV boot path with no C bit set in the page-tables. All data written to memory would then be unencrypted and could leak sensitive data to the hypervisor. Add sanity checks to the 32-bit boot #VC handler to make sure the hypervisor does not pretend that SEV is not enabled. Signed-off-by: Joerg Roedel --- arch/x86/boot/compressed/mem_encrypt.S | 28 ++ 1 file changed, 28 insertions(+) diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S index e915f4906477..0211eddefeb0 100644 --- a/arch/x86/boot/compressed/mem_encrypt.S +++ b/arch/x86/boot/compressed/mem_encrypt.S @@ -139,6 +139,26 @@ SYM_CODE_START(startup32_vc_handler) jnz .Lfail movl%edx, 0(%esp) # Store result + /* +* Sanity check CPUID results from the Hypervisor. See comment in +* do_vc_no_ghcb() for more details on why this is necessary. +*/ + + /* Fail if SEV leaf not available in CPUID[0x8000].EAX */ + cmpl$0x8000, %ebx + jne .Lcheck_sev + cmpl$0x801f, 12(%esp) + jb .Lfail + jmp .Ldone + +.Lcheck_sev: + /* Fail if SEV bit not set in CPUID[0x801f].EAX[1] */ + cmpl$0x801f, %ebx + jne .Ldone + btl $1, 12(%esp) + jnc .Lfail + +.Ldone: popl%edx popl%ecx popl%ebx @@ -152,6 +172,14 @@ SYM_CODE_START(startup32_vc_handler) iret .Lfail: + /* Send terminate request to Hypervisor */ + movl$0x100, %eax + xorl%edx, %edx + movl$MSR_AMD64_SEV_ES_GHCB, %ecx + wrmsr + rep; vmmcall + + /* If request fails, go to hlt loop */ hlt jmp .Lfail SYM_CODE_END(startup32_vc_handler) -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 0/8] x86/seves: Support 32-bit boot path and other updates
From: Joerg Roedel Hi, these patches add support for the 32-bit boot in the decompressor code. This is needed to boot an SEV-ES guest on some firmware and grub versions. The patches also add the necessary CPUID sanity checks and a 32-bit version of the C-bit check. Other updates included here: 1. Add code to shut down exception handling in the decompressor code before jumping to the real kernel. Once in the real kernel it is not safe anymore to jump back to the decompressor code via exceptions. 2. Replace open-coded hlt loops with proper calls to sev_es_terminate(). Please review. Thanks, Joerg Changes v2->v3: - Added a patch to remove the check for the Hypervisor CPUID bit for detecting SEV Changes v1->v2: - Addressed Boris' review comments. - Fixed a bug which caused the cbit-check to never be executed even in an SEV guest. Joerg Roedel (8): x86/boot/compressed/64: Cleanup exception handling before booting kernel x86/sev: Do not require Hypervisor CPUID bit for SEV guests x86/boot/compressed/64: Reload CS in startup_32 x86/boot/compressed/64: Setup IDT in startup_32 boot path x86/boot/compressed/64: Add 32-bit boot #VC handler x86/boot/compressed/64: Add CPUID sanity check to 32-bit boot-path x86/boot/compressed/64: Check SEV encryption in 32-bit boot-path x86/sev-es: Replace open-coded hlt-loops with sev_es_terminate() arch/x86/boot/compressed/head_64.S | 170 - arch/x86/boot/compressed/idt_64.c | 14 ++ arch/x86/boot/compressed/mem_encrypt.S | 130 ++- arch/x86/boot/compressed/misc.c| 7 +- arch/x86/boot/compressed/misc.h| 6 + arch/x86/boot/compressed/sev-es.c | 12 +- arch/x86/kernel/sev-es-shared.c| 16 +-- arch/x86/mm/mem_encrypt_identity.c | 35 ++--- 8 files changed, 340 insertions(+), 50 deletions(-) -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 1/8] x86/boot/compressed/64: Cleanup exception handling before booting kernel
From: Joerg Roedel
Disable the exception handling before booting the kernel to make sure
any exceptions that happen during early kernel boot are not directed to
the pre-decompression code.
Signed-off-by: Joerg Roedel
---
arch/x86/boot/compressed/idt_64.c | 14 ++
arch/x86/boot/compressed/misc.c | 7 ++-
arch/x86/boot/compressed/misc.h | 6 ++
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/arch/x86/boot/compressed/idt_64.c
b/arch/x86/boot/compressed/idt_64.c
index 804a502ee0d2..9b93567d663a 100644
--- a/arch/x86/boot/compressed/idt_64.c
+++ b/arch/x86/boot/compressed/idt_64.c
@@ -52,3 +52,17 @@ void load_stage2_idt(void)
load_boot_idt(_idt_desc);
}
+
+void cleanup_exception_handling(void)
+{
+ /*
+* Flush GHCB from cache and map it encrypted again when running as
+* SEV-ES guest.
+*/
+ sev_es_shutdown_ghcb();
+
+ /* Set a null-idt, disabling #PF and #VC handling */
+ boot_idt_desc.size= 0;
+ boot_idt_desc.address = 0;
+ load_boot_idt(_idt_desc);
+}
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 267e7f93050e..cc9fd0e8766a 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -443,11 +443,8 @@ asmlinkage __visible void *extract_kernel(void *rmode,
memptr heap,
handle_relocations(output, output_len, virt_addr);
debug_putstr("done.\nBooting the kernel.\n");
- /*
-* Flush GHCB from cache and map it encrypted again when running as
-* SEV-ES guest.
-*/
- sev_es_shutdown_ghcb();
+ /* Disable exception handling before booting the kernel */
+ cleanup_exception_handling();
return output;
}
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 901ea5ebec22..e5612f035498 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -155,6 +155,12 @@ extern pteval_t __default_kernel_pte_mask;
extern gate_desc boot_idt[BOOT_IDT_ENTRIES];
extern struct desc_ptr boot_idt_desc;
+#ifdef CONFIG_X86_64
+void cleanup_exception_handling(void);
+#else
+static inline void cleanup_exception_handling(void) { }
+#endif
+
/* IDT Entry Points */
void boot_page_fault(void);
void boot_stage1_vc(void);
--
2.30.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 3/8] x86/boot/compressed/64: Reload CS in startup_32
From: Joerg Roedel Exception handling in the startup_32 boot path requires the CS selector to be correctly set up. Reload it from the current GDT. Signed-off-by: Joerg Roedel --- arch/x86/boot/compressed/head_64.S | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index e94874f4bbc1..c59c80ca546d 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -107,9 +107,16 @@ SYM_FUNC_START(startup_32) movl%eax, %gs movl%eax, %ss -/* setup a stack and make sure cpu supports long mode. */ + /* Setup a stack and load CS from current GDT */ lealrva(boot_stack_end)(%ebp), %esp + pushl $__KERNEL32_CS + lealrva(1f)(%ebp), %eax + pushl %eax + lretl +1: + + /* Make sure cpu supports long mode. */ callverify_cpu testl %eax, %eax jnz .Lno_longmode -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: linux-next: Tree for Mar 11 [drivers/vdpa/virtio_pci/vp_vdpa.ko]
On 2021/3/12 2:18 上午, Randy Dunlap wrote:
On 3/10/21 9:14 PM, Stephen Rothwell wrote:
Hi all,
Warning: Some of the branches in linux-next are still based on v5.12-rc1,
so please be careful if you are trying to bisect a bug.
News: if your -next included tree is based on Linus' tree tag
v5.12-rc1{,-dontuse} (or somewhere between v5.11 and that tag), please
consider rebasing it onto v5.12-rc2. Also, please check any branches
merged into your branch.
Changes since 20210310:
on i386:
ERROR: modpost: "vp_modern_get_queue_notify_off"
[drivers/vdpa/virtio_pci/vp_vdpa.ko] undefined!
ERROR: modpost: "vp_modern_get_num_queues" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_probe" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_queue_address" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_set_queue_size" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_set_queue_enable"
[drivers/vdpa/virtio_pci/vp_vdpa.ko] undefined!
ERROR: modpost: "vp_modern_get_queue_enable"
[drivers/vdpa/virtio_pci/vp_vdpa.ko] undefined!
ERROR: modpost: "vp_modern_get_features" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_set_features" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_set_status" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_get_status" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_config_vector" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_queue_vector" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_generation" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
ERROR: modpost: "vp_modern_remove" [drivers/vdpa/virtio_pci/vp_vdpa.ko]
undefined!
Full randconfig file is attached.
Fix sent.
Thanks.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7] i2c: virtio: add a virtio i2c frontend driver
On Fri, Mar 12, 2021 at 2:33 PM Jie Deng wrote:
> +
> +/**
> + * struct virtio_i2c_req - the virtio I2C request structure
> + * @out_hdr: the OUT header of the virtio I2C message
> + * @buf: the buffer into which data is read, or from which it's written
> + * @in_hdr: the IN header of the virtio I2C message
> + */
> +struct virtio_i2c_req {
> + struct virtio_i2c_out_hdr out_hdr;
> + uint8_t *buf;
> + struct virtio_i2c_in_hdr in_hdr;
> +};
The simpler request structure clearly looks better than the previous version,
but I think I found another problem here, at least a theoretical one:
When you map the headers into the DMA address space, they should
be in separate cache lines, to allow the DMA mapping interfaces to
perform cache management on each one without accidentally clobbering
another member.
So far I think there is an assumption that virtio buffers are always
on cache-coherent devices, but if you ever have a virtio-i2c device
backend on a physical interconnect that is not cache coherent (e.g. a
microcontroller that shares the memory bus), this breaks down.
You could avoid this by either allocating arrays of each type separately,
or by marking each member that you pass to the device as
cacheline_aligned.
Arnd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7] i2c: virtio: add a virtio i2c frontend driver
On 2021/3/12 16:11, Viresh Kumar wrote:
On 12-03-21, 15:51, Jie Deng wrote:
On 2021/3/12 14:10, Viresh Kumar wrote:
I saw your email about wrong version being sent, I already wrote some
reviews. Sending them anyway for FWIW :)
On 12-03-21, 21:33, Jie Deng wrote:
+struct virtio_i2c {
+ struct virtio_device *vdev;
+ struct completion completion;
+ struct i2c_adapter adap;
+ struct mutex lock;
As I said in the previous version (Yes, we were both waiting for
Wolfram to answer that), this lock shouldn't be required at all.
And since none of us have a use-case at hand where we will have a
problem without this lock, we should really skip it. We can always
come back and add it if we find an issue somewhere. Until then, better
to keep it simple.
The problem is you can't guarantee that adap->algo->master_xfer
is only called from i2c_transfer. Any function who holds the adapter can
call
adap->algo->master_xfer directly.
See my last reply here, (almost) no one in the mainline kernel call it
directly. And perhaps you can consider the caller broken in that case
and so there is no need of an extra lock, unless you have a case that
is broken.
https://lore.kernel.org/lkml/20210305072903.wtw645rukmqr5hx5@vireshk-i7/
I prefer to avoid potential issues rather
than
find a issue then fix.
This is a very hypothetical issue IMHO as the kernel code doesn't have
such a user. There is no need of locks here, else the i2c core won't
have handled it by itself.
I'd like to see Wolfram's opinion.
Is it safe to remove lock in adap->algo->master_xfer ?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
