Re: [PATCH 1/2] vdpa: mlx5: prevent cvq work from hogging CPU

[Jason Wang]

On Thu, Mar 24, 2022 at 8:54 AM Hillf Danton  wrote:
>
> On Tue, 22 Mar 2022 09:59:14 +0800 Jason Wang wrote:
> >
> > Yes, there will be no "infinite" loop, but since the loop is triggered
> > by userspace. It looks to me it will delay the flush/drain of the
> > workqueue forever which is still suboptimal.
>
> Usually it is barely possible to shoot two birds using a stone.
>
> Given the "forever", I am inclined to not running faster, hehe, though
> another cobble is to add another line in the loop checking if mvdev is
> unregistered, and for example make mvdev->cvq unready before destroying
> workqueue.
>
> static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct 
> vdpa_device *dev)
> {
> struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct 
> mlx5_vdpa_mgmtdev, mgtdev);
> struct mlx5_vdpa_dev *mvdev = to_mvdev(dev);
> struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>
> mlx5_notifier_unregister(mvdev->mdev, >nb);
> destroy_workqueue(mvdev->wq);
> _vdpa_unregister_device(dev);
> mgtdev->ndev = NULL;
> }
>

Yes, so we had

1) using a quota for re-requeue
2) using something like

while (READ_ONCE(cvq->ready)) {
...
cond_resched();
}

There should not be too much difference except we need to use
cancel_work_sync() instead of flush_work for 1).

I would keep the code as is but if you stick I can change.

Thanks

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Re: Re: [PATCH v3 0/6] Support akcipher for virtio-crypto

[zhenwei pi]

On 3/24/22 02:03, Eric Biggers wrote:

On Wed, Mar 23, 2022 at 03:32:37PM +0800, zhenwei pi wrote:


On 3/23/22 13:17, Eric Biggers wrote:

On Wed, Mar 23, 2022 at 10:49:06AM +0800, zhenwei pi wrote:

v2 -> v3:
- Introduce akcipher types to qapi
- Add test/benchmark suite for akcipher class
- Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
- crypto: Introduce akcipher crypto class
- virtio-crypto: Introduce RSA algorithm

v1 -> v2:
- Update virtio_crypto.h from v2 version of related kernel patch.

v1:
- Support akcipher for virtio-crypto.
- Introduce akcipher class.
- Introduce ASN1 decoder into QEMU.
- Implement RSA backend by nettle/hogweed.

Lei He (3):
crypto-akcipher: Introduce akcipher types to qapi
crypto: Implement RSA algorithm by hogweed
tests/crypto: Add test suite for crypto akcipher

Zhenwei Pi (3):
virtio-crypto: header update
crypto: Introduce akcipher crypto class
virtio-crypto: Introduce RSA algorithm


You forgot to describe the point of this patchset and what its use case is.
Like any other Linux kernel patchset, that needs to be in the cover letter.

- Eric

Thanks Eric for pointing this missing part.

This feature provides akcipher service offloading capability. QEMU side
handles asymmetric requests via virtio-crypto devices from guest side, do
encrypt/decrypt/sign/verify operations on host side, and return the result
to guest.

This patchset implements a RSA backend by hogweed from nettle, it works
together with guest patch:
https://lkml.org/lkml/2022/3/1/1425


So what is the use case?

- Eric

Hi,

In our plan, the feature is designed for HTTPS offloading case and other 
applications which use kernel RSA/ecdsa by keyctl syscall. The full 
picture shows bellow:



  Nginx/openssl[1] ... Apps
Guest   -
   virtio-crypto driver[2]
-
   virtio-crypto backend[3]
Host-
  /  |  \
  builtin[4]   vhost keyctl[5] ...


[1] User applications can offload RSA calculation to kernel by keyctl 
syscall. There is no keyctl engine in openssl currently, we developed a 
engine and tried to contribute it to openssl upstream, but openssl 1.x 
does not accept new feature. Link:

https://github.com/openssl/openssl/pull/16689

This branch is available and maintained by Lei 
https://github.com/TousakaRin/openssl/tree/OpenSSL_1_1_1-kctl_engine

We tested nginx(change config file only) with openssl keyctl engine, it 
works fine.


[2] virtio-crypto driver is used to communicate with host side, send 
requests to host side to do asymmetric calculation.

https://lkml.org/lkml/2022/3/1/1425

[3] virtio-crypto backend handles requests from guest side, and forwards 
request to crypto backend driver of QEMU.


[4] Currently RSA is supported only in builtin driver. This driver is 
supposed to test the full feature without other software(Ex vhost 
process) and hardware dependence. ecdsa is introduced into qapi type 
without implementation, this may be implemented in Q3-2022 or later. If 
ecdsa type definition should be added with the implementation together, 
I'll remove this in next version.


[5] keyctl backend is in development, we will post this feature in 
Q2-2022. keyctl backend can use hardware acceleration(Ex, Intel QAT).


Setup the full environment, tested with Intel QAT on host side, the QPS 
of HTTPS increase to ~200% in a guest.


VS PCI passthrough: the most important benefit of this solution makes 
the VM migratable.


--
zhenwei pi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/2] vdpa: mlx5: prevent cvq work from hogging CPU

[Hillf Danton]

On Tue, 22 Mar 2022 09:59:14 +0800 Jason Wang wrote:
> 
> Yes, there will be no "infinite" loop, but since the loop is triggered
> by userspace. It looks to me it will delay the flush/drain of the
> workqueue forever which is still suboptimal.

Usually it is barely possible to shoot two birds using a stone.

Given the "forever", I am inclined to not running faster, hehe, though
another cobble is to add another line in the loop checking if mvdev is
unregistered, and for example make mvdev->cvq unready before destroying
workqueue.

static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device 
*dev)
{
struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct 
mlx5_vdpa_mgmtdev, mgtdev);
struct mlx5_vdpa_dev *mvdev = to_mvdev(dev);
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);

mlx5_notifier_unregister(mvdev->mdev, >nb);
destroy_workqueue(mvdev->wq);
_vdpa_unregister_device(dev);
mgtdev->ndev = NULL;
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net v3 0/3] vsock/virtio: enable VQs early on probe and finish the setup before using them

[Michael S. Tsirkin]

On Wed, Mar 23, 2022 at 06:36:22PM +0100, Stefano Garzarella wrote:
> The first patch fixes a virtio-spec violation. The other two patches
> complete the driver configuration before using the VQs in the probe.
> 
> The patch order should simplify backporting in stable branches.


Series:

Acked-by: Michael S. Tsirkin 

> v3:
> - re-ordered the patch to improve bisectability [MST]
> 
> v2: https://lore.kernel.org/netdev/20220323084954.11769-1-sgarz...@redhat.com/
> v1: https://lore.kernel.org/netdev/20220322103823.83411-1-sgarz...@redhat.com/
> 
> Stefano Garzarella (3):
>   vsock/virtio: initialize vdev->priv before using VQs
>   vsock/virtio: read the negotiated features before using VQs
>   vsock/virtio: enable VQs early on probe
> 
>  net/vmw_vsock/virtio_transport.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> -- 
> 2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v3 3/3] vsock/virtio: enable VQs early on probe

[Stefano Garzarella]

virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, but virtio-vsock
driver uses VQs in the probe function to fill rx and event VQs
with new buffers.

Let's fix this, calling virtio_device_ready() before using VQs
in the probe function.

Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 3954d3be9083..ba1c8cc0c467 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -627,6 +627,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 
vdev->priv = vsock;
 
+   virtio_device_ready(vdev);
+
mutex_lock(>tx_lock);
vsock->tx_run = true;
mutex_unlock(>tx_lock);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v3 2/3] vsock/virtio: read the negotiated features before using VQs

[Stefano Garzarella]

Complete the driver configuration, reading the negotiated features,
before using the VQs in the virtio_vsock_probe().

Fixes: 53efbba12cc7 ("virtio/vsock: enable SEQPACKET for transport")
Suggested-by: Michael S. Tsirkin 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 3e5513934c9f..3954d3be9083 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -622,6 +622,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
INIT_WORK(>event_work, virtio_transport_event_work);
INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
 
+   if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
+   vsock->seqpacket_allow = true;
+
vdev->priv = vsock;
 
mutex_lock(>tx_lock);
@@ -638,9 +641,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
vsock->event_run = true;
mutex_unlock(>event_lock);
 
-   if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
-   vsock->seqpacket_allow = true;
-
rcu_assign_pointer(the_virtio_vsock, vsock);
 
mutex_unlock(_virtio_vsock_mutex);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v3 1/3] vsock/virtio: initialize vdev->priv before using VQs

[Stefano Garzarella]

When we fill VQs with empty buffers and kick the host, it may send
an interrupt. `vdev->priv` must be initialized before this since it
is used in the virtqueue callbacks.

Fixes: 0deab087b16a ("vsock/virtio: use RCU to avoid use-after-free on 
the_virtio_vsock")
Suggested-by: Michael S. Tsirkin 
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5afc194a58bb..3e5513934c9f 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -622,6 +622,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
INIT_WORK(>event_work, virtio_transport_event_work);
INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
 
+   vdev->priv = vsock;
+
mutex_lock(>tx_lock);
vsock->tx_run = true;
mutex_unlock(>tx_lock);
@@ -639,7 +641,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
vsock->seqpacket_allow = true;
 
-   vdev->priv = vsock;
rcu_assign_pointer(the_virtio_vsock, vsock);
 
mutex_unlock(_virtio_vsock_mutex);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v3 0/3] vsock/virtio: enable VQs early on probe and finish the setup before using them

[Stefano Garzarella]

The first patch fixes a virtio-spec violation. The other two patches
complete the driver configuration before using the VQs in the probe.

The patch order should simplify backporting in stable branches.

v3:
- re-ordered the patch to improve bisectability [MST]

v2: https://lore.kernel.org/netdev/20220323084954.11769-1-sgarz...@redhat.com/
v1: https://lore.kernel.org/netdev/20220322103823.83411-1-sgarz...@redhat.com/

Stefano Garzarella (3):
  vsock/virtio: initialize vdev->priv before using VQs
  vsock/virtio: read the negotiated features before using VQs
  vsock/virtio: enable VQs early on probe

 net/vmw_vsock/virtio_transport.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Re: [PATCH v3 1/6] virtio-crypto: header update

[zhenwei pi]

On 3/23/22 23:38, Daniel P. Berrangé wrote:

On Wed, Mar 23, 2022 at 10:49:07AM +0800, zhenwei pi wrote:

Update header from linux, support akcipher service.


I'm assuming this is updated for *non-merged* Linux headers, since
I don't see these changes present in current linux.git




Hi,

The related context link:
https://lkml.org/lkml/2022/3/1/1425

- The virtio crypto spec is the first part. It will be deferred to 1.3.
The latest version: 
https://www.oasis-open.org/committees/ballot.php?id=3681 (need put 
"__le32 akcipher_algo;" instead of "__le32 reserve;" and repost)


- According to the spec, then we can define the linux headers. (depend 
on the spec)


- Update the header file for QEMU. (depend on the linux headers)

All the parts are in development.

--
zhenwei pi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 1/6] virtio-crypto: header update

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:07AM +0800, zhenwei pi wrote:
> Update header from linux, support akcipher service.

I'm assuming this is updated for *non-merged* Linux headers, since
I don't see these changes present in current linux.git 

> 
> Reviewed-by: Gonglei 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  .../standard-headers/linux/virtio_crypto.h| 82 ++-
>  1 file changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/include/standard-headers/linux/virtio_crypto.h 
> b/include/standard-headers/linux/virtio_crypto.h
> index 5ff0b4ee59..68066dafb6 100644
> --- a/include/standard-headers/linux/virtio_crypto.h
> +++ b/include/standard-headers/linux/virtio_crypto.h
> @@ -37,6 +37,7 @@
>  #define VIRTIO_CRYPTO_SERVICE_HASH   1
>  #define VIRTIO_CRYPTO_SERVICE_MAC2
>  #define VIRTIO_CRYPTO_SERVICE_AEAD   3
> +#define VIRTIO_CRYPTO_SERVICE_AKCIPHER 4
>  
>  #define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
>  
> @@ -57,6 +58,10 @@ struct virtio_crypto_ctrl_header {
>  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
>  #define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
>  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> +#define VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION \
> +VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x04)
> +#define VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION \
> +VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x05)
>   uint32_t opcode;
>   uint32_t algo;
>   uint32_t flag;
> @@ -180,6 +185,58 @@ struct virtio_crypto_aead_create_session_req {
>   uint8_t padding[32];
>  };
>  
> +struct virtio_crypto_rsa_session_para {
> +#define VIRTIO_CRYPTO_RSA_RAW_PADDING   0
> +#define VIRTIO_CRYPTO_RSA_PKCS1_PADDING 1
> + uint32_t padding_algo;
> +
> +#define VIRTIO_CRYPTO_RSA_NO_HASH   0
> +#define VIRTIO_CRYPTO_RSA_MD2   1
> +#define VIRTIO_CRYPTO_RSA_MD3   2
> +#define VIRTIO_CRYPTO_RSA_MD4   3
> +#define VIRTIO_CRYPTO_RSA_MD5   4
> +#define VIRTIO_CRYPTO_RSA_SHA1  5

Do we really need to be adding support for all these obsolete
hash functions. Maybe SHA1 is borderline acceptable, but all
those obsolete MD* functions too ??

> +#define VIRTIO_CRYPTO_RSA_SHA2566
> +#define VIRTIO_CRYPTO_RSA_SHA3847
> +#define VIRTIO_CRYPTO_RSA_SHA5128
> +#define VIRTIO_CRYPTO_RSA_SHA2249
> + uint32_t hash_algo;
> +};
> +
> +struct virtio_crypto_ecdsa_session_para {
> +#define VIRTIO_CRYPTO_CURVE_UNKNOWN   0
> +#define VIRTIO_CRYPTO_CURVE_NIST_P192 1
> +#define VIRTIO_CRYPTO_CURVE_NIST_P224 2
> +#define VIRTIO_CRYPTO_CURVE_NIST_P256 3
> +#define VIRTIO_CRYPTO_CURVE_NIST_P384 4
> +#define VIRTIO_CRYPTO_CURVE_NIST_P521 5
> + uint32_t curve_id;
> + uint32_t padding;
> +};
> +
> +struct virtio_crypto_akcipher_session_para {
> +#define VIRTIO_CRYPTO_NO_AKCIPHER0
> +#define VIRTIO_CRYPTO_AKCIPHER_RSA   1
> +#define VIRTIO_CRYPTO_AKCIPHER_DSA   2
> +#define VIRTIO_CRYPTO_AKCIPHER_ECDSA 3

Here we have RSA, DSA and ECDSA, but the corresponding QEMU
qapi/crypto.json doesn't define DSA at all. Is that a mistake
on the QEMU side, or is the DSA support redundant ?

> + uint32_t algo;
> +
> +#define VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PUBLIC  1
> +#define VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PRIVATE 2
> + uint32_t keytype;
> + uint32_t keylen;
> +
> + union {
> + struct virtio_crypto_rsa_session_para rsa;
> + struct virtio_crypto_ecdsa_session_para ecdsa;
> + } u;
> +};


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 5/6] tests/crypto: Add test suite for crypto akcipher

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:11AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add unit test and benchmark test for crypto akcipher.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  tests/bench/benchmark-crypto-akcipher.c | 163 ++
>  tests/bench/meson.build |   6 +
>  tests/bench/test_akcipher_keys.inc  | 277 +
>  tests/unit/meson.build  |   1 +
>  tests/unit/test-crypto-akcipher.c   | 715 
>  5 files changed, 1162 insertions(+)
>  create mode 100644 tests/bench/benchmark-crypto-akcipher.c
>  create mode 100644 tests/bench/test_akcipher_keys.inc
>  create mode 100644 tests/unit/test-crypto-akcipher.c
> 
> diff --git a/tests/bench/benchmark-crypto-akcipher.c 
> b/tests/bench/benchmark-crypto-akcipher.c
> new file mode 100644
> index 00..152fed8d73
> --- /dev/null
> +++ b/tests/bench/benchmark-crypto-akcipher.c
> @@ -0,0 +1,163 @@
> +/*
> + * QEMU Crypto cipher speed benchmark
> + *
> + * Copyright (c) 2022 Bytedance
> + *
> + * Authors:
> + *lei he 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "crypto/init.h"
> +#include "crypto/akcipher.h"
> +#include "standard-headers/linux/virtio_crypto.h"
> +
> +#include "test_akcipher_keys.inc"
> +
> +static bool keep_running;
> +
> +static void alarm_handler(int sig)
> +{
> +keep_running = false;
> +}
> +
> +static QCryptoAkcipher *create_rsa_akcipher(const uint8_t *priv_key,
> +size_t keylen,
> +QCryptoRsaPaddingAlgorithm 
> padding,
> +QCryptoRsaHashAlgorithm hash)
> +{
> +QCryptoRsaOptions opt;
> +QCryptoAkcipher *rsa;
> +Error *err = NULL;
> +
> +opt.padding_algo = padding;
> +opt.hash_algo = hash;
> +rsa = qcrypto_akcipher_new(QCRYPTO_AKCIPHER_ALG_RSA,
> +   QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE,
> +   priv_key, keylen, , );
> +
> +g_assert(rsa != NULL);
> +return rsa;
> +}
> +
> +static void test_rsa_speed(const uint8_t *priv_key, size_t keylen,
> +   size_t key_size)
> +{
> +#define Byte 8
> +#define SHA1_DGST_LEN 40
> +#define DURATION_SECONDS 10
> +#define padding QCRYPTO_RSA_PADDING_ALG_PKCS1
> +#define hash QCRYPTO_RSA_HASH_ALG_SHA1

'Byte' 'padding' and 'duration' are 

> +
> +Error *err = NULL;
> +QCryptoAkcipher *rsa;
> +uint8_t *dgst, *signature;
> +size_t count;
> +
> +rsa = create_rsa_akcipher(priv_key, keylen, padding, hash);
> +
> +dgst = g_new0(uint8_t, SHA1_DGST_LEN);
> +memset(dgst, g_test_rand_int(), SHA1_DGST_LEN);
> +signature = g_new0(uint8_t, key_size / Byte);
> +
> +g_test_message("benchmark rsa%lu (%s-%s) sign in %d seconds", key_size,
> +   QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   DURATION_SECONDS);
> +alarm(DURATION_SECONDS);
> +g_test_timer_start();
> +for (keep_running = true, count = 0; keep_running; ++count) {
> +g_assert(qcrypto_akcipher_sign(rsa, dgst, SHA1_DGST_LEN,
> +   signature, key_size / Byte, ) > 
> 0);
> +}
> +g_test_timer_elapsed();



> +g_test_message("rsa%lu (%s-%s) sign %lu times in %.2f seconds,"
> +   " %.2f times/sec ",
> +   key_size,  QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   count, g_test_timer_last(),
> +   (double)count / g_test_timer_last());
> +
> +g_test_message("benchmark rsa%lu (%s-%s) verify in %d seconds", key_size,
> +   QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   DURATION_SECONDS);
> +alarm(DURATION_SECONDS);
> +g_test_timer_start();
> +for (keep_running = true, count = 0; keep_running; ++count) {
> +g_assert(qcrypto_akcipher_verify(rsa, signature, key_size / Byte,
> + dgst, SHA1_DGST_LEN, ) == 0);
> +}
> +g_test_timer_elapsed();
> +g_test_message("rsa%lu (%s-%s) verify %lu times in %.2f seconds,"
> +   " %.2f times/sec ",
> +   key_size, QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   count, g_test_timer_last(),
> +   (double)count / g_test_timer_last());
> +
> +g_assert(qcrypto_akcipher_free(rsa, ) == 0);
> +g_free(dgst);
> +g_free(signature);
> +}
> +
> +static void test_rsa_1024_speed(const void *opaque)
> +{
> +size_t key_size = (size_t)opaque;
> +

Re: [PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:08AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce akcipher types, also include RSA & ECDSA related types.
> 
> Signed-off-by: Lei He 
> Signed-off-by: zhenwei pi 
> ---
>  qapi/crypto.json | 86 
>  1 file changed, 86 insertions(+)
> 
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 1ec54c15ca..d44c38e3b1 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -540,3 +540,89 @@
>'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
>  '*sanity-check': 'bool',
>  '*passwordid': 'str' } }
> +##
> +# @QCryptoAkcipherAlgorithm:
> +#
> +# The supported algorithms for asymmetric encryption ciphers
> +#
> +# @rsa: RSA algorithm
> +# @ecdsa: ECDSA algorithm
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherAlgorithm',
> +  'prefix': 'QCRYPTO_AKCIPHER_ALG',
> +  'data': ['rsa', 'ecdsa']}

What were your intentions wrt  ecdsa - the nettle impl in this patch
series doesn't appear to actually support ecdsa. Are you intending to
add this in later versions of this patch series, or do it as separate
work at a later date ?


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Re: [PATCH v3 0/6] Support akcipher for virtio-crypto

[zhenwei pi]

On 3/23/22 20:36, Michael S. Tsirkin wrote:

On Wed, Mar 23, 2022 at 10:49:06AM +0800, zhenwei pi wrote:

v2 -> v3:
- Introduce akcipher types to qapi
- Add test/benchmark suite for akcipher class
- Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
   - crypto: Introduce akcipher crypto class
   - virtio-crypto: Introduce RSA algorithm


Thanks!
I tagged this but qemu is in freeze. If possible pls ping or
repost after the release to help make sure I don't lose it.


Hi,

Daniel has started to review this patchset, according to Daniel's 
important suggestion, I'll rework this feature and post the next version 
later.


Thanks a lot!

--
zhenwei pi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net v2 1/3] vsock/virtio: enable VQs early on probe

[Stefano Garzarella]

On Wed, Mar 23, 2022 at 01:44:42PM +, Stefan Hajnoczi wrote:

On Wed, Mar 23, 2022 at 09:49:52AM +0100, Stefano Garzarella wrote:

virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, but virtio-vsock
driver uses VQs in the probe function to fill rx and event VQs
with new buffers.

Let's fix this, calling virtio_device_ready() before using VQs
in the probe function.

Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5afc194a58bb..b1962f8cd502 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -622,6 +622,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
INIT_WORK(>event_work, virtio_transport_event_work);
INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);

+   virtio_device_ready(vdev);


Can rx and event virtqueue interrupts be lost if they occur before we
assign vdev->priv later in virtio_vsock_probe()?


Yep, as Michael suggested I'll fix the patch order in the next version.

Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 4/6] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:10AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce ASN.1 decoder, and implement RSA algorithm by hogweed
> from nettle. Thus QEMU supports a 'real' RSA backend to handle
> request from guest side. It's important to test RSA offload case
> without OS & hardware requirement.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher-nettle.c  | 523 ++
>  crypto/akcipher.c |   3 +
>  crypto/asn1_decoder.c | 185 ++
>  crypto/asn1_decoder.h |  42 +++

Please introduce the asn1 files in a separate commit, and also
provide a unit test to validate them in the same commit.

> diff --git a/crypto/akcipher-nettle.c b/crypto/akcipher-nettle.c
> new file mode 100644
> index 00..45b93af772
> --- /dev/null
> +++ b/crypto/akcipher-nettle.c
> @@ -0,0 +1,523 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + *
> + */
> +
> +#include 
> +
> +#include 
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "asn1_decoder.h"
> +#include "crypto/akcipher.h"
> +#include "crypto/random.h"
> +#include "qapi/error.h"
> +#include "sysemu/cryptodev.h"
> +
> +typedef struct QCryptoNettleRsa {
> +QCryptoAkcipher akcipher;
> +struct rsa_public_key pub;
> +struct rsa_private_key priv;
> +QCryptoRsaPaddingAlgorithm padding_algo;
> +QCryptoRsaHashAlgorithm hash_algo;
> +} QCryptoNettleRsa;

Call this QCryptoAkCipherNettleRSA

> +
> +struct asn1_parse_ctx {
> +const uint8_t *data;
> +size_t dlen;
> +};
> +
> +#define Octet 8
> +
> +static int extract_value(void *p, const uint8_t *data, size_t dlen)
> +{
> +struct asn1_parse_ctx *ctx = (struct asn1_parse_ctx *)p;
> +ctx->data = (uint8_t *)data;
> +ctx->dlen = dlen;
> +
> +return 0;
> +}
> +
> +static int extract_mpi(void *p, const uint8_t *data, size_t dlen)
> +{
> +mpz_t *target = (mpz_t *)p;
> +nettle_mpz_set_str_256_u(*target, dlen, data);
> +
> +return 0;
> +}
> +
> +static QCryptoNettleRsa *qcrypto_nettle_rsa_malloc(void);
> +
> +static void qcrypto_nettle_rsa_destroy(void *ptr)
> +{
> +QCryptoNettleRsa *rsa = (QCryptoNettleRsa *)ptr;
> +if (!rsa) {
> +return;
> +}
> +
> +rsa_public_key_clear(>pub);
> +rsa_private_key_clear(>priv);
> +g_free(rsa);
> +}
> +
> +static QCryptoAkcipher *qcrypto_nettle_new_rsa(
> +QCryptoAkcipherKeyType type,
> +const uint8_t *key,  size_t keylen,
> +QCryptoRsaOptions *opt, Error **errp);
> +
> +QCryptoAkcipher *qcrypto_akcipher_nettle_new(QCryptoAkcipherAlgorithm alg,
> + QCryptoAkcipherKeyType type,
> + const uint8_t *key,
> + size_t keylen, void *para,
> + Error **errp)
> +{
> +switch (alg) {
> +case QCRYPTO_AKCIPHER_ALG_RSA:
> +return qcrypto_nettle_new_rsa(type, key, keylen,
> +  (QCryptoRsaOptions *)para, errp);
> +default:
> +error_setg(errp, "Unsupported algorithm: %u", alg);
> +return NULL;
> +}
> +
> +return NULL;
> +}
> +
> +/**
> + * Parse ber encoded rsa private key, asn1 schema:
> + *RsaPrivKey ::= SEQUENCE {
> + * version INTEGER
> + * n   INTEGER
> + * e   INTEGER
> + * d   INTEGER
> + * p   INTEGER
> + * q   INTEGER
> + * e1  INTEGER
> + * e2  INTEGER
> + * u   INTEGER
> + * }
> + */
> +static int parse_rsa_private_key(QCryptoNettleRsa *rsa,
> + const uint8_t *key, size_t keylen)
> +{
> +struct asn1_parse_ctx ctx;
> +
> +if (ber_decode_seq(, , extract_value, ) != 0 ||
> +keylen != 0) {
> +return -1;
> +}
> +
> +if (ber_decode_int(, , NULL, NULL) != 0 ||
> +ber_decode_int(, , extract_mpi, >pub.n) != 0 
> ||
> +ber_decode_int(, , extract_mpi, >pub.e) != 0 
> ||
> +ber_decode_int(, , extract_mpi, >priv.d) != 

Re: [PATCH net v2 3/3] vsock/virtio: read the negotiated features before using VQs

[Stefan Hajnoczi]

On Wed, Mar 23, 2022 at 09:49:54AM +0100, Stefano Garzarella wrote:
> Complete the driver configuration, reading the negotiated features,
> before using the VQs and tell the device that the driver is ready in
> the virtio_vsock_probe().
> 
> Fixes: 53efbba12cc7 ("virtio/vsock: enable SEQPACKET for transport")
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Stefano Garzarella 
> ---
>  net/vmw_vsock/virtio_transport.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net v2 2/3] vsock/virtio: initialize vdev->priv before using VQs

[Stefan Hajnoczi]

On Wed, Mar 23, 2022 at 09:49:53AM +0100, Stefano Garzarella wrote:
> When we fill VQs with empty buffers and kick the host, it may send
> an interrupt. `vdev->priv` must be initialized before this since it
> is used in the virtqueue callback.
> 
> Fixes: 0deab087b16a ("vsock/virtio: use RCU to avoid use-after-free on 
> the_virtio_vsock")
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Stefano Garzarella 
> ---
>  net/vmw_vsock/virtio_transport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport.c 
> b/net/vmw_vsock/virtio_transport.c
> index b1962f8cd502..fff67ad39087 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -622,6 +622,7 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
>   INIT_WORK(>event_work, virtio_transport_event_work);
>   INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
>  
> + vdev->priv = vsock;
>   virtio_device_ready(vdev);

Doh, patch order got me. :)

Stefan


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net v2 1/3] vsock/virtio: enable VQs early on probe

[Stefan Hajnoczi]

On Wed, Mar 23, 2022 at 09:49:52AM +0100, Stefano Garzarella wrote:
> virtio spec requires drivers to set DRIVER_OK before using VQs.
> This is set automatically after probe returns, but virtio-vsock
> driver uses VQs in the probe function to fill rx and event VQs
> with new buffers.
> 
> Let's fix this, calling virtio_device_ready() before using VQs
> in the probe function.
> 
> Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")
> Signed-off-by: Stefano Garzarella 
> ---
>  net/vmw_vsock/virtio_transport.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/vmw_vsock/virtio_transport.c 
> b/net/vmw_vsock/virtio_transport.c
> index 5afc194a58bb..b1962f8cd502 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -622,6 +622,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
>   INIT_WORK(>event_work, virtio_transport_event_work);
>   INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
>  
> + virtio_device_ready(vdev);

Can rx and event virtqueue interrupts be lost if they occur before we
assign vdev->priv later in virtio_vsock_probe()?

Stefan


signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net v2 0/3] vsock/virtio: enable VQs early on probe and finish the setup before using them

[Stefano Garzarella]

On Wed, Mar 23, 2022 at 09:22:02AM -0400, Michael S. Tsirkin wrote:

On Wed, Mar 23, 2022 at 09:49:51AM +0100, Stefano Garzarella wrote:

The first patch fixes a virtio-spec violation. The other two patches
complete the driver configuration before using the VQs in the probe.

The patch order should simplify backporting in stable branches.


Ok but I think the order is wrong. It should be 2-3-1,
otherwise bisect can pick just 1 and it will have
the issues previous reviw pointed out.


Right, I prioritized simplifying the backport, but obviously 
bisectability is priority!


I'll send v3 changing the order in 2-3-1

Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 3/6] crypto: Introduce akcipher crypto class

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:09AM +0800, zhenwei pi wrote:
> Support basic asymmetric operations: encrypt, decrypt, sign and
> verify.
> 
> Co-developed-by: lei he 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher.c |  78 +
>  crypto/meson.build|   1 +
>  include/crypto/akcipher.h | 139 ++
>  3 files changed, 218 insertions(+)
>  create mode 100644 crypto/akcipher.c
>  create mode 100644 include/crypto/akcipher.h
> 
> diff --git a/crypto/akcipher.c b/crypto/akcipher.c
> new file mode 100644
> index 00..1e52f2fd76
> --- /dev/null
> +++ b/crypto/akcipher.c
> @@ -0,0 +1,78 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: zhenwei pi 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "qapi/error.h"
> +#include "crypto/akcipher.h"
> +
> +QCryptoAkcipher *qcrypto_akcipher_new(QCryptoAkcipherAlgorithm alg,
> +  QCryptoAkcipherKeyType type,
> +  const uint8_t *key, size_t keylen,
> +  void *para, Error **errp)
> +{
> +QCryptoAkcipher *akcipher = NULL;
> +
> +return akcipher;
> +}
> +
> +int qcrypto_akcipher_encrypt(QCryptoAkcipher *akcipher,
> + const void *data, size_t data_len,
> + void *enc, size_t enc_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->encrypt(akcipher, data, data_len, enc, enc_len, errp);
> +}
> +
> +int qcrypto_akcipher_decrypt(struct QCryptoAkcipher *akcipher,
> + const void *enc, size_t enc_len,
> + void *data, size_t data_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->decrypt(akcipher, enc, enc_len, data, data_len, errp);
> +}
> +
> +int qcrypto_akcipher_sign(struct QCryptoAkcipher *akcipher,
> +  const void *data, size_t data_len,
> +  void *sig, size_t sig_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->sign(akcipher, data, data_len, sig, sig_len, errp);
> +}
> +
> +int qcrypto_akcipher_verify(struct QCryptoAkcipher *akcipher,
> +const void *sig, size_t sig_len,
> +const void *data, size_t data_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->verify(akcipher, sig, sig_len, data, data_len, errp);
> +}
> +
> +int qcrypto_akcipher_free(struct QCryptoAkcipher *akcipher, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->free(akcipher, errp);
> +}
> diff --git a/crypto/meson.build b/crypto/meson.build
> index 19c44bea89..c32b57aeda 100644
> --- a/crypto/meson.build
> +++ b/crypto/meson.build
> @@ -19,6 +19,7 @@ crypto_ss.add(files(
>'tlscredspsk.c',
>'tlscredsx509.c',
>'tlssession.c',
> +  'akcipher.c',
>  ))
>  
>  if nettle.found()
> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> new file mode 100644
> index 00..03cc3bf46b
> --- /dev/null
> +++ b/include/crypto/akcipher.h
> @@ -0,0 +1,139 @@
> +/*
> + * QEMU Crypto asymmetric algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: zhenwei pi 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + *
> + */
> +
> +#ifndef QCRYPTO_AKCIPHER_H

Re: [PATCH v2] virtio: pci: sanity check bar indexes

[Michael S. Tsirkin]

On Wed, Mar 23, 2022 at 01:21:55PM +, Keir Fraser wrote:
> On Wed, Mar 23, 2022 at 08:01:42AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Mar 23, 2022 at 03:57:59PM +0800, Jason Wang wrote:
> > > On Tue, Mar 22, 2022 at 11:20 PM Keir Fraser  wrote:
> > > >
> > > > The bar index is used as an index into the device's resource list
> > > > and should be checked as within range for a standard bar.
> > > >
> > > > Also clean up an existing check to consistently use PCI_STD_NUM_BARS.
> > > >
> > > > Signed-off-by: Keir Fraser 
> > > > ---
> > > >  drivers/virtio/virtio_pci_modern.c | 10 --
> > > >  drivers/virtio/virtio_pci_modern_dev.c |  8 +++-
> > > >  2 files changed, 15 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/virtio/virtio_pci_modern.c 
> > > > b/drivers/virtio/virtio_pci_modern.c
> > > > index 5455bc041fb6..84bace98dff5 100644
> > > > --- a/drivers/virtio/virtio_pci_modern.c
> > > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > > @@ -293,7 +293,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > > > *dev, u8 required_id,
> > > >
> > > > for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR); pos > 0;
> > > >  pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) 
> > > > {
> > > > -   u8 type, cap_len, id;
> > > > +   u8 type, cap_len, id, res_bar;
> > > > u32 tmp32;
> > > > u64 res_offset, res_length;
> > > >
> > > > @@ -317,7 +317,12 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > > > *dev, u8 required_id,
> > > >
> > > > /* Type, and ID match, looks good */
> > > > pci_read_config_byte(dev, pos + offsetof(struct 
> > > > virtio_pci_cap,
> > > > -bar), bar);
> > > > +bar), 
> > > > _bar);
> > > > +   if (res_bar >= PCI_STD_NUM_BARS) {
> > > > +   dev_err(>dev, "%s: shm cap with bad bar: 
> > > > %d\n",
> > > > +   __func__, res_bar);
> > > > +   continue;
> > > > +   }
> > > >
> > > > /* Read the lower 32bit of length and offset */
> > > > pci_read_config_dword(dev, pos + offsetof(struct 
> > > > virtio_pci_cap,
> > 
> > In fact, the spec says such BAR values are reserved, not bad, so
> > the capabiluty should be ignored, they should not cause the driver to error 
> > out
> > or print errors.
> 
> Ah yes, so I see. It makes sense then to silently ignore the capability and 
> print nothing.
> I will fix it.
> 
> > > > @@ -337,6 +342,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > > > *dev, u8 required_id,
> > > >  length_hi), 
> > > > );
> > > > res_length |= ((u64)tmp32) << 32;
> > > >
> > > > +   *bar = res_bar;
> > > > *offset = res_offset;
> > > > *len = res_length;
> > > >
> > > > diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
> > > > b/drivers/virtio/virtio_pci_modern_dev.c
> > > > index e8b3ff2b9fbc..a6911d1e212a 100644
> > > > --- a/drivers/virtio/virtio_pci_modern_dev.c
> > > > +++ b/drivers/virtio/virtio_pci_modern_dev.c
> > > > @@ -35,6 +35,12 @@ vp_modern_map_capability(struct 
> > > > virtio_pci_modern_device *mdev, int off,
> > > > pci_read_config_dword(dev, off + offsetof(struct 
> > > > virtio_pci_cap, length),
> > > >   );
> > > >
> > > > +   if (bar >= PCI_STD_NUM_BARS) {
> > > > +   dev_err(>dev,
> > > > +   "virtio_pci: bad capability bar %u\n", bar);
> > 
> > In fact, I would say the issue is less that bar is reserved.
> > The real issue is that the value apparently changed since
> > we read it the first time. I think it's a good idea to
> > reflect that in the message. Maybe find_capability should return
> > the capability structure so we don't need to re-read it from
> > the device?
> 
> I will have a look and fix it up one way or the other, and respin
> this patch.
> 
>  Thanks,
>   Keir

BTW avoiding extra reads is good for start up speed. This is slow path,
but still.

> > > > +   return NULL;
> > > > +   }
> > > > +
> > > > if (length <= start) {
> > > > dev_err(>dev,
> > > > "virtio_pci: bad capability len %u (>%u 
> > > > expected)\n",
> > > > @@ -120,7 +126,7 @@ static inline int virtio_pci_find_capability(struct 
> > > > pci_dev *dev, u8 cfg_type,
> > > >  );
> > > >
> > > > /* Ignore structures with reserved BAR values */
> > > > -   if (bar > 0x5)
> > > > +   if (bar >= PCI_STD_NUM_BARS)
> > > > continue;
> > > 
> > > Just notice that the spec said:
> > > 
> > > "
> > > values 0x0 to 0x5 specify a Base 

Re: [PATCH net v2 0/3] vsock/virtio: enable VQs early on probe and finish the setup before using them

[Michael S. Tsirkin]

On Wed, Mar 23, 2022 at 09:49:51AM +0100, Stefano Garzarella wrote:
> The first patch fixes a virtio-spec violation. The other two patches
> complete the driver configuration before using the VQs in the probe.
> 
> The patch order should simplify backporting in stable branches.

Ok but I think the order is wrong. It should be 2-3-1,
otherwise bisect can pick just 1 and it will have
the issues previous reviw pointed out.



> v2:
> - patch 1 is not changed from v1
> - added 2 patches to complete the driver configuration before using the
>   VQs in the probe [MST]
> 
> v1: https://lore.kernel.org/netdev/20220322103823.83411-1-sgarz...@redhat.com/
> 
> Stefano Garzarella (3):
>   vsock/virtio: enable VQs early on probe
>   vsock/virtio: initialize vdev->priv before using VQs
>   vsock/virtio: read the negotiated features before using VQs
> 
>  net/vmw_vsock/virtio_transport.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> -- 
> 2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:08AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce akcipher types, also include RSA & ECDSA related types.
> 
> Signed-off-by: Lei He 
> Signed-off-by: zhenwei pi 
> ---
>  qapi/crypto.json | 86 
>  1 file changed, 86 insertions(+)
> 
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 1ec54c15ca..d44c38e3b1 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -540,3 +540,89 @@
>'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
>  '*sanity-check': 'bool',
>  '*passwordid': 'str' } }
> +##
> +# @QCryptoAkcipherAlgorithm:

Should be named  QCryptoAkCipherAlgorithm

> +#
> +# The supported algorithms for asymmetric encryption ciphers
> +#
> +# @rsa: RSA algorithm
> +# @ecdsa: ECDSA algorithm
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherAlgorithm',
> +  'prefix': 'QCRYPTO_AKCIPHER_ALG',
> +  'data': ['rsa', 'ecdsa']}
> +
> +##
> +# @QCryptoAkcipherKeyType:

Should be named  QCryptoAkCipherKeyType

> +#
> +# The type of asymmetric keys.
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherKeyType',
> +  'prefix': 'QCRYPTO_AKCIPHER_KEY_TYPE',
> +  'data': ['public', 'private']}
> +
> +##
> +# @QCryptoRsaHashAlgorithm:
> +#
> +# The hash algorithm for RSA pkcs1 padding algothrim
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoRsaHashAlgorithm',
> +  'prefix': 'QCRYPTO_RSA_HASH_ALG',
> +  'data': [ 'md2', 'md3', 'md4', 'md5', 'sha1', 'sha256', 'sha384', 
> 'sha512', 'sha224' ]}

We already have QCryptoHashAlgorithm and I don't see the
benefit in duplicating it here.

We don't have md2, md3, and md4 in QCryptoHashAlgorithm, but
that doesn't look like a real negative as I can't imagine
those should be used today.

> +##
> +# @QCryptoRsaPaddingAlgorithm:
> +#
> +# The padding algorithm for RSA.
> +#
> +# @raw: no padding used
> +# @pkcs1: pkcs1#v1.5
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoRsaPaddingAlgorithm',
> +  'prefix': 'QCRYPTO_RSA_PADDING_ALG',
> +  'data': ['raw', 'pkcs1']}
> +
> +##
> +# @QCryptoCurveId:

Should be named  QCryptoCurveID

> +#
> +# The well-known curves, referenced from 
> https://csrc.nist.gov/csrc/media/publications/fips/186/3/archive/2009-06-25/documents/fips_186-3.pdf
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoCurveId',
> +  'prefix': 'QCRYPTO_CURVE_ID',
> +  'data': ['nist-p192', 'nist-p224', 'nist-p256', 'nist-p384', 'nist-p521']}


> +
> +##
> +# @QCryptoRsaOptions:

This should be named  QCryptoAkCipherOptionsRSA

> +#
> +# Specific parameters for RSA algorithm.
> +#
> +# @hash-algo: QCryptoRsaHashAlgorithm
> +# @padding-algo: QCryptoRsaPaddingAlgorithm
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'QCryptoRsaOptions',
> +  'data': { 'hash-algo':'QCryptoRsaHashAlgorithm',
> +'padding-algo': 'QCryptoRsaPaddingAlgorithm'}}

Our naming convention is  'XXX-alg' rather than 'XXX-algo'.

> +
> +##
> +# @QCryptoEcdsaOptions:

This should be named  QCryptoAkCipherOptionsECDSA

> +#
> +# Specific parameter for ECDSA algorithm.
> +#
> +# @curve-id: QCryptoCurveId
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'QCryptoEcdsaOptions',
> +  'data': { 'curve-id': 'QCryptoCurveId' }}

Having these two structs standalone looks wrong to me. I suspect that
callers will need to be able to conditionally pass in either one, and
so require the API to use a discriminated union

  { 'union': 'QCryptoAkCipherOptions'
'base': { 'algorithm': 'QCryptoAkCipherAlgorithm' },
'discriminator': 'algorithm',
'data': { 'rsa': 'QCryptoAkCipherOptionsRSA' ,
  'ecdsa': 'QCryptoAkCipherOptionsECDSA' } }


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 0/6] Support akcipher for virtio-crypto

[Michael S. Tsirkin]

On Wed, Mar 23, 2022 at 10:49:06AM +0800, zhenwei pi wrote:
> v2 -> v3:
> - Introduce akcipher types to qapi
> - Add test/benchmark suite for akcipher class
> - Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
>   - crypto: Introduce akcipher crypto class
>   - virtio-crypto: Introduce RSA algorithm

Thanks!
I tagged this but qemu is in freeze. If possible pls ping or
repost after the release to help make sure I don't lose it.

> v1 -> v2:
> - Update virtio_crypto.h from v2 version of related kernel patch.
> 
> v1:
> - Support akcipher for virtio-crypto.
> - Introduce akcipher class.
> - Introduce ASN1 decoder into QEMU.
> - Implement RSA backend by nettle/hogweed.
> 
> Lei He (3):
>   crypto-akcipher: Introduce akcipher types to qapi
>   crypto: Implement RSA algorithm by hogweed
>   tests/crypto: Add test suite for crypto akcipher
> 
> Zhenwei Pi (3):
>   virtio-crypto: header update
>   crypto: Introduce akcipher crypto class
>   virtio-crypto: Introduce RSA algorithm
> 
>  backends/cryptodev-builtin.c  | 319 +++-
>  backends/cryptodev-vhost-user.c   |  34 +-
>  backends/cryptodev.c  |  32 +-
>  crypto/akcipher-nettle.c  | 523 +
>  crypto/akcipher.c |  81 ++
>  crypto/asn1_decoder.c | 185 +
>  crypto/asn1_decoder.h |  42 +
>  crypto/meson.build|   4 +
>  hw/virtio/virtio-crypto.c | 326 ++--
>  include/crypto/akcipher.h | 155 
>  include/hw/virtio/virtio-crypto.h |   5 +-
>  .../standard-headers/linux/virtio_crypto.h|  82 +-
>  include/sysemu/cryptodev.h|  88 ++-
>  meson.build   |  11 +
>  qapi/crypto.json  |  86 +++
>  tests/bench/benchmark-crypto-akcipher.c   | 163 
>  tests/bench/meson.build   |   6 +
>  tests/bench/test_akcipher_keys.inc| 277 +++
>  tests/unit/meson.build|   1 +
>  tests/unit/test-crypto-akcipher.c | 715 ++
>  20 files changed, 2990 insertions(+), 145 deletions(-)
>  create mode 100644 crypto/akcipher-nettle.c
>  create mode 100644 crypto/akcipher.c
>  create mode 100644 crypto/asn1_decoder.c
>  create mode 100644 crypto/asn1_decoder.h
>  create mode 100644 include/crypto/akcipher.h
>  create mode 100644 tests/bench/benchmark-crypto-akcipher.c
>  create mode 100644 tests/bench/test_akcipher_keys.inc
>  create mode 100644 tests/unit/test-crypto-akcipher.c
> 
> -- 
> 2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2] virtio: pci: sanity check bar indexes

[Michael S. Tsirkin]

On Wed, Mar 23, 2022 at 03:57:59PM +0800, Jason Wang wrote:
> On Tue, Mar 22, 2022 at 11:20 PM Keir Fraser  wrote:
> >
> > The bar index is used as an index into the device's resource list
> > and should be checked as within range for a standard bar.
> >
> > Also clean up an existing check to consistently use PCI_STD_NUM_BARS.
> >
> > Signed-off-by: Keir Fraser 
> > ---
> >  drivers/virtio/virtio_pci_modern.c | 10 --
> >  drivers/virtio/virtio_pci_modern_dev.c |  8 +++-
> >  2 files changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_modern.c 
> > b/drivers/virtio/virtio_pci_modern.c
> > index 5455bc041fb6..84bace98dff5 100644
> > --- a/drivers/virtio/virtio_pci_modern.c
> > +++ b/drivers/virtio/virtio_pci_modern.c
> > @@ -293,7 +293,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, 
> > u8 required_id,
> >
> > for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR); pos > 0;
> >  pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> > -   u8 type, cap_len, id;
> > +   u8 type, cap_len, id, res_bar;
> > u32 tmp32;
> > u64 res_offset, res_length;
> >
> > @@ -317,7 +317,12 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > *dev, u8 required_id,
> >
> > /* Type, and ID match, looks good */
> > pci_read_config_byte(dev, pos + offsetof(struct 
> > virtio_pci_cap,
> > -bar), bar);
> > +bar), _bar);
> > +   if (res_bar >= PCI_STD_NUM_BARS) {
> > +   dev_err(>dev, "%s: shm cap with bad bar: %d\n",
> > +   __func__, res_bar);
> > +   continue;
> > +   }
> >
> > /* Read the lower 32bit of length and offset */
> > pci_read_config_dword(dev, pos + offsetof(struct 
> > virtio_pci_cap,

In fact, the spec says such BAR values are reserved, not bad, so
the capabiluty should be ignored, they should not cause the driver to error out
or print errors.

> > @@ -337,6 +342,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, 
> > u8 required_id,
> >  length_hi), );
> > res_length |= ((u64)tmp32) << 32;
> >
> > +   *bar = res_bar;
> > *offset = res_offset;
> > *len = res_length;
> >
> > diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
> > b/drivers/virtio/virtio_pci_modern_dev.c
> > index e8b3ff2b9fbc..a6911d1e212a 100644
> > --- a/drivers/virtio/virtio_pci_modern_dev.c
> > +++ b/drivers/virtio/virtio_pci_modern_dev.c
> > @@ -35,6 +35,12 @@ vp_modern_map_capability(struct virtio_pci_modern_device 
> > *mdev, int off,
> > pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, 
> > length),
> >   );
> >
> > +   if (bar >= PCI_STD_NUM_BARS) {
> > +   dev_err(>dev,
> > +   "virtio_pci: bad capability bar %u\n", bar);

In fact, I would say the issue is less that bar is reserved.
The real issue is that the value apparently changed since
we read it the first time. I think it's a good idea to
reflect that in the message. Maybe find_capability should return
the capability structure so we don't need to re-read it from
the device?

> > +   return NULL;
> > +   }
> > +
> > if (length <= start) {
> > dev_err(>dev,
> > "virtio_pci: bad capability len %u (>%u 
> > expected)\n",
> > @@ -120,7 +126,7 @@ static inline int virtio_pci_find_capability(struct 
> > pci_dev *dev, u8 cfg_type,
> >  );
> >
> > /* Ignore structures with reserved BAR values */
> > -   if (bar > 0x5)
> > +   if (bar >= PCI_STD_NUM_BARS)
> > continue;
> 
> Just notice that the spec said:
> 
> "
> values 0x0 to 0x5 specify a Base Address register (BAR) belonging to
> the function located beginning at 10h in PCI Configuration Space and
> used to map the structure into Memory or I/O Space. The BAR is
> permitted to be either 32-bit or 64-bit, it can map Memory Space or
> I/O Space.
> 
> Any other value is reserved for future use.
> "
> So we probably need to stick 0x5 instead of 0x6 (PCI_STD_NUM_BARS) for
> this and other places.
> 
> Thanks

It does not matter much IMHO, the reason spec uses 0 to 0x5 is precisely
because that's the standard number of BARs. Both ways work as long as we
are consistent, and I guess PCI_STD_NUM_BARS might be preferable since
people tend to copy paste values.


> >
> > if (type == cfg_type) {
> > --
> > 2.35.1.894.gb6a874cedc-goog
> >

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org

Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Robin Murphy]

On 2022-03-23 10:11, Gerd Hoffmann wrote:

On Wed, Mar 23, 2022 at 09:45:13AM +, Robin Murphy wrote:

On 2022-03-23 07:15, Christian K�nig wrote:

Am 22.03.22 um 10:34 schrieb Cong Liu:

qxl use ioremap to map ram_header and rom, in the arm64 implementation,
the device is mapped as DEVICE_nGnRE, it can not support unaligned
access.


Well that some ARM boards doesn't allow unaligned access to MMIO space
is a well known bug of those ARM boards.

So as far as I know this is a hardware bug you are trying to workaround
here and I'm not 100% sure that this is correct.


No, this one's not a bug. The Device memory type used for iomem mappings is
*architecturally* defined to enforce properties like aligned accesses, no
speculation, no reordering, etc. If something wants to be treated more like
RAM than actual MMIO registers, then ioremap_wc() or ioremap_cache() is the
appropriate thing to do in general (with the former being a bit more
portable according to Documentation/driver-api/device-io.rst).


Well, qxl is a virtual device, so it *is* ram.

I'm wondering whenever qxl actually works on arm?  As far I know all
virtual display devices with (virtual) pci memory bars for vram do not
work on arm due to the guest mapping vram as io memory and the host
mapping vram as normal ram and the mapping attribute mismatch causes
caching troubles (only noticeable on real arm hardware, not in
emulation).  Did something change here recently?


Indeed, Armv8.4 introduced the S2FWB feature to cope with situations 
like this - essentially it allows the hypervisor to share RAM-backed 
pages with the guest without losing coherency regardless of how the 
guest maps them.


Robin.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: 回复: Re: 回复: Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Christian König via Virtualization]

Hi Cong,

yes I've seen that, but that is still not sufficient.

You need to update the check in ttm_module.c as well or otherwise your 
userspace mapping might not work correctly either.


Regards,
Christian.

Am 23.03.22 um 11:00 schrieb liuco...@kylinos.cn:


Hi Christian,


another commit fix the case in ttm. I send two patches at the same 
time, but seems I miss


 '--cover-letter' when run format-patch or some other bad operation.




Regards,

Cong.




*主　题：*Re: 回复: Re: [PATCH v1 1/2] drm/qxl: replace ioremap by 
ioremap_cache on arm64

*日　期：*2022-03-23 17:34
*发件人：*Christian König
*收件人：*liucong2@kylinos.cnairlied@redhat.comkraxel@redhat.comairlied@linux.iedaniel@ffwll.chray.huang@amd.comvirtualization@lists.linux-foundation.orgspice-devel@lists.freedesktop.orgdri-de...@lists.freedesktop.org 



Hi Cong,

   well than Dave must decide what to do here.

   When QXL emulates a device it should also not use memory accesses   
 which won't work on a physical device.


   BTW: Your patch is really buggy, it misses the cases in ttm_module.c

   Regards,
   Christian.

Am 23.03.22 um 09:48 schrieb liuco...@kylinos.cn:


Hi Christian,


according to 'Arm Architecture Reference Manual Armv8,for        Armv8-A

architecture profile' pdf E2.6.5


E2.6.5 Generation of Alignment faults by load/store multiple       
 accesses to


 Device memory


When FEAT_LSMAOC is        implemented and the value of the 
applicable nTLSMD


field is 0, any memory        access by an AArch32 Load Multiple or 
Store


Multiple instruction to        an address that the stage 1 translation

assigns as Device-nGRE,        Device-nGnRE, or Device-nGnRnE generates

an Alignment fault.


so it seems not just some ARM boards doesn't allow unaligned       
 access to MMIO


space, all pci memory mapped as Device-nGnRE in arm64 cannot       
 support


unaligned access. and qxl is a device simulated by qemu, it       
 should be able to access


to MMIO space in a more flexible way(PROT_NORMAL) than the real       
 physical


graphics card.






Cong.





*主　题：*Re: [PATCH v1 1/2]          drm/qxl: replace ioremap by 
ioremap_cache on arm64

*日　期：*2022-03-23 15:15
*发件人：*Christian König
*收件人：*CongLiuairlied@redhat.comkraxel@redhat.comairlied@linux.iedaniel@ffwll.chray.huang@amd.comvirtualization@lists.linux-foundation.orgspice-devel@lists.freedesktop.orgdri-de...@lists.freedesktop.org 




Am 22.03.22 um 10:34 schrieb Cong Liu:
       > qxl use ioremap to map ram_header and rom, in the arm64     
   implementation,
       > the device is mapped as DEVICE_nGnRE, it can not support     
   unaligned

       > access.

       Well that some ARM boards doesn't allow unaligned access to 
MMIO        space

       is a well known bug of those ARM boards.

       So as far as I know this is a hardware bug you are trying to   
     workaround

       here and I'm not 100% sure that this is correct.

       Christian.

       >
       >    6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
       > [    6.620961] lr : setup_slot+0x34/0xf0 [qxl]
       > [    6.621376] sp : 800012b73760
       > [    6.621701] x29: 800012b73760 x28: 0001   
     x27: 1000
       > [    6.622400] x26: 0001 x25: 0400   
     x24: cf376848c000
       > [    6.623099] x23: c4087400 x22: cf3718e17140   
     x21: 
       > [    6.623823] x20: c4086000 x19: c40870b0   
     x18: 0014
       > [    6.624519] x17: 4d3605ab x16: bb3b6129   
     x15: 6e771809
       > [    6.625214] x14: 0001 x13: 007473696c5f7974   
     x12: 696e69615f65
       > [    6.625909] x11: d543656a x10:    
     x9 : cf3718e085a4
       > [    6.626616] x8 : 006c7871 x7 : 000a   
     x6 : 0017
       > [    6.627343] x5 : 1400 x4 : 800011f63400   
     x3 : 1400
       > [    6.628047] x2 :  x1 : c40870b0   
     x0 : c4086000

       > [    6.628751] Call trace:
       > [    6.628994]  setup_hw_slot+0x24/0x60 [qxl]
       > [    6.629404]  setup_slot+0x34/0xf0 [qxl]
       > [    6.629790]  qxl_device_init+0x6f0/0x7f0 [qxl]
       > [    6.630235]  qxl_pci_probe+0xdc/0x1d0 [qxl]
       > [    6.630654]  local_pci_probe+0x48/0xb8
       > [    6.631027]  pci_device_probe+0x194/0x208
       > [    6.631464]  really_probe+0xd0/0x458
       > [    6.631818]  __driver_probe_device+0x124/0x1c0
       > [    6.632256]  driver_probe_device+0x48/0x130
       > [    6.632669]  __driver_attach+0xc4/0x1a8
       > [    6.633049]  bus_for_each_dev+0x78/0xd0
       > [    6.633437]  driver_attach+0x2c/0x38
       > [    6.633789]  bus_add_driver+0x154/0x248
       > [    6.634168]  driver_register+0x6c/0x128
       > [    6.635205]  __pci_register_driver+0x4c/0x58
       > [    6.635628]  qxl_init+0x48/0x1000 [qxl]
       > [    

Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Gerd Hoffmann]

On Wed, Mar 23, 2022 at 09:45:13AM +, Robin Murphy wrote:
> On 2022-03-23 07:15, Christian König wrote:
> > Am 22.03.22 um 10:34 schrieb Cong Liu:
> > > qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> > > the device is mapped as DEVICE_nGnRE, it can not support unaligned
> > > access.
> > 
> > Well that some ARM boards doesn't allow unaligned access to MMIO space
> > is a well known bug of those ARM boards.
> > 
> > So as far as I know this is a hardware bug you are trying to workaround
> > here and I'm not 100% sure that this is correct.
> 
> No, this one's not a bug. The Device memory type used for iomem mappings is
> *architecturally* defined to enforce properties like aligned accesses, no
> speculation, no reordering, etc. If something wants to be treated more like
> RAM than actual MMIO registers, then ioremap_wc() or ioremap_cache() is the
> appropriate thing to do in general (with the former being a bit more
> portable according to Documentation/driver-api/device-io.rst).

Well, qxl is a virtual device, so it *is* ram.

I'm wondering whenever qxl actually works on arm?  As far I know all
virtual display devices with (virtual) pci memory bars for vram do not
work on arm due to the guest mapping vram as io memory and the host
mapping vram as normal ram and the mapping attribute mismatch causes
caching troubles (only noticeable on real arm hardware, not in
emulation).  Did something change here recently?

take care,
  Gerd

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Christian König via Virtualization]

Am 23.03.22 um 10:45 schrieb Robin Murphy:

On 2022-03-23 07:15, Christian König wrote:

Am 22.03.22 um 10:34 schrieb Cong Liu:

qxl use ioremap to map ram_header and rom, in the arm64 implementation,
the device is mapped as DEVICE_nGnRE, it can not support unaligned
access.


Well that some ARM boards doesn't allow unaligned access to MMIO 
space is a well known bug of those ARM boards.


So as far as I know this is a hardware bug you are trying to 
workaround here and I'm not 100% sure that this is correct.


No, this one's not a bug. The Device memory type used for iomem 
mappings is *architecturally* defined to enforce properties like 
aligned accesses, no speculation, no reordering, etc. If something 
wants to be treated more like RAM than actual MMIO registers, then 
ioremap_wc() or ioremap_cache() is the appropriate thing to do in 
general (with the former being a bit more portable according to 
Documentation/driver-api/device-io.rst).


Of course *then* you might find that on some systems the 
interconnect/PCIe implementation/endpoint doesn't actually like 
unaligned accesses once the CPU MMU starts allowing them to be sent 
out, but hey, one step at a time ;)


Ah, good point! I was already wondering why that sometimes work and 
sometimes doesn't.


Thanks,
Christian.



Robin.



Christian.



   6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
[    6.620961] lr : setup_slot+0x34/0xf0 [qxl]
[    6.621376] sp : 800012b73760
[    6.621701] x29: 800012b73760 x28: 0001 x27: 
1000
[    6.622400] x26: 0001 x25: 0400 x24: 
cf376848c000
[    6.623099] x23: c4087400 x22: cf3718e17140 x21: 

[    6.623823] x20: c4086000 x19: c40870b0 x18: 
0014
[    6.624519] x17: 4d3605ab x16: bb3b6129 x15: 
6e771809
[    6.625214] x14: 0001 x13: 007473696c5f7974 x12: 
696e69615f65
[    6.625909] x11: d543656a x10:  x9 : 
cf3718e085a4
[    6.626616] x8 : 006c7871 x7 : 000a x6 : 
0017
[    6.627343] x5 : 1400 x4 : 800011f63400 x3 : 
1400
[    6.628047] x2 :  x1 : c40870b0 x0 : 
c4086000

[    6.628751] Call trace:
[    6.628994]  setup_hw_slot+0x24/0x60 [qxl]
[    6.629404]  setup_slot+0x34/0xf0 [qxl]
[    6.629790]  qxl_device_init+0x6f0/0x7f0 [qxl]
[    6.630235]  qxl_pci_probe+0xdc/0x1d0 [qxl]
[    6.630654]  local_pci_probe+0x48/0xb8
[    6.631027]  pci_device_probe+0x194/0x208
[    6.631464]  really_probe+0xd0/0x458
[    6.631818]  __driver_probe_device+0x124/0x1c0
[    6.632256]  driver_probe_device+0x48/0x130
[    6.632669]  __driver_attach+0xc4/0x1a8
[    6.633049]  bus_for_each_dev+0x78/0xd0
[    6.633437]  driver_attach+0x2c/0x38
[    6.633789]  bus_add_driver+0x154/0x248
[    6.634168]  driver_register+0x6c/0x128
[    6.635205]  __pci_register_driver+0x4c/0x58
[    6.635628]  qxl_init+0x48/0x1000 [qxl]
[    6.636013]  do_one_initcall+0x50/0x240
[    6.636390]  do_init_module+0x60/0x238
[    6.636768]  load_module+0x2458/0x2900
[    6.637136]  __do_sys_finit_module+0xbc/0x128
[    6.637561]  __arm64_sys_finit_module+0x28/0x38
[    6.638004]  invoke_syscall+0x74/0xf0
[    6.638366]  el0_svc_common.constprop.0+0x58/0x1a8
[    6.638836]  do_el0_svc+0x2c/0x90
[    6.639216]  el0_svc+0x40/0x190
[    6.639526]  el0t_64_sync_handler+0xb0/0xb8
[    6.639934]  el0t_64_sync+0x1a4/0x1a8
[    6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
[    6.640889] ---[ end trace 95615d89b7c87f95 ]---

Signed-off-by: Cong Liu 
---
  drivers/gpu/drm/qxl/qxl_kms.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c 
b/drivers/gpu/drm/qxl/qxl_kms.c

index 4dc5ad13f12c..0e61ac04d8ad 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
   (int)qdev->surfaceram_size / 1024,
   (sb == 4) ? "64bit" : "32bit");
+#ifdef CONFIG_ARM64
+    qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
+#else
  qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
+#endif
  if (!qdev->rom) {
  pr_err("Unable to ioremap ROM\n");
  r = -ENOMEM;
@@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
  goto rom_unmap;
  }
+#ifdef CONFIG_ARM64
+    qdev->ram_header = ioremap_cache(qdev->vram_base +
+   qdev->rom->ram_header_offset,
+   sizeof(*qdev->ram_header));
+#else
  qdev->ram_header = ioremap(qdev->vram_base +
 qdev->rom->ram_header_offset,
 sizeof(*qdev->ram_header));
+#endif
  if (!qdev->ram_header) {
  DRM_ERROR("Unable to ioremap RAM header\n");
  r = -ENOMEM;




___
Virtualization mailing list

Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Robin Murphy]

On 2022-03-23 07:15, Christian König wrote:

Am 22.03.22 um 10:34 schrieb Cong Liu:

qxl use ioremap to map ram_header and rom, in the arm64 implementation,
the device is mapped as DEVICE_nGnRE, it can not support unaligned
access.


Well that some ARM boards doesn't allow unaligned access to MMIO space 
is a well known bug of those ARM boards.


So as far as I know this is a hardware bug you are trying to workaround 
here and I'm not 100% sure that this is correct.


No, this one's not a bug. The Device memory type used for iomem mappings 
is *architecturally* defined to enforce properties like aligned 
accesses, no speculation, no reordering, etc. If something wants to be 
treated more like RAM than actual MMIO registers, then ioremap_wc() or 
ioremap_cache() is the appropriate thing to do in general (with the 
former being a bit more portable according to 
Documentation/driver-api/device-io.rst).


Of course *then* you might find that on some systems the 
interconnect/PCIe implementation/endpoint doesn't actually like 
unaligned accesses once the CPU MMU starts allowing them to be sent out, 
but hey, one step at a time ;)


Robin.



Christian.



   6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
[    6.620961] lr : setup_slot+0x34/0xf0 [qxl]
[    6.621376] sp : 800012b73760
[    6.621701] x29: 800012b73760 x28: 0001 x27: 
1000
[    6.622400] x26: 0001 x25: 0400 x24: 
cf376848c000
[    6.623099] x23: c4087400 x22: cf3718e17140 x21: 

[    6.623823] x20: c4086000 x19: c40870b0 x18: 
0014
[    6.624519] x17: 4d3605ab x16: bb3b6129 x15: 
6e771809
[    6.625214] x14: 0001 x13: 007473696c5f7974 x12: 
696e69615f65
[    6.625909] x11: d543656a x10:  x9 : 
cf3718e085a4
[    6.626616] x8 : 006c7871 x7 : 000a x6 : 
0017
[    6.627343] x5 : 1400 x4 : 800011f63400 x3 : 
1400
[    6.628047] x2 :  x1 : c40870b0 x0 : 
c4086000

[    6.628751] Call trace:
[    6.628994]  setup_hw_slot+0x24/0x60 [qxl]
[    6.629404]  setup_slot+0x34/0xf0 [qxl]
[    6.629790]  qxl_device_init+0x6f0/0x7f0 [qxl]
[    6.630235]  qxl_pci_probe+0xdc/0x1d0 [qxl]
[    6.630654]  local_pci_probe+0x48/0xb8
[    6.631027]  pci_device_probe+0x194/0x208
[    6.631464]  really_probe+0xd0/0x458
[    6.631818]  __driver_probe_device+0x124/0x1c0
[    6.632256]  driver_probe_device+0x48/0x130
[    6.632669]  __driver_attach+0xc4/0x1a8
[    6.633049]  bus_for_each_dev+0x78/0xd0
[    6.633437]  driver_attach+0x2c/0x38
[    6.633789]  bus_add_driver+0x154/0x248
[    6.634168]  driver_register+0x6c/0x128
[    6.635205]  __pci_register_driver+0x4c/0x58
[    6.635628]  qxl_init+0x48/0x1000 [qxl]
[    6.636013]  do_one_initcall+0x50/0x240
[    6.636390]  do_init_module+0x60/0x238
[    6.636768]  load_module+0x2458/0x2900
[    6.637136]  __do_sys_finit_module+0xbc/0x128
[    6.637561]  __arm64_sys_finit_module+0x28/0x38
[    6.638004]  invoke_syscall+0x74/0xf0
[    6.638366]  el0_svc_common.constprop.0+0x58/0x1a8
[    6.638836]  do_el0_svc+0x2c/0x90
[    6.639216]  el0_svc+0x40/0x190
[    6.639526]  el0t_64_sync_handler+0xb0/0xb8
[    6.639934]  el0t_64_sync+0x1a4/0x1a8
[    6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
[    6.640889] ---[ end trace 95615d89b7c87f95 ]---

Signed-off-by: Cong Liu 
---
  drivers/gpu/drm/qxl/qxl_kms.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c 
b/drivers/gpu/drm/qxl/qxl_kms.c

index 4dc5ad13f12c..0e61ac04d8ad 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
   (int)qdev->surfaceram_size / 1024,
   (sb == 4) ? "64bit" : "32bit");
+#ifdef CONFIG_ARM64
+    qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
+#else
  qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
+#endif
  if (!qdev->rom) {
  pr_err("Unable to ioremap ROM\n");
  r = -ENOMEM;
@@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
  goto rom_unmap;
  }
+#ifdef CONFIG_ARM64
+    qdev->ram_header = ioremap_cache(qdev->vram_base +
+   qdev->rom->ram_header_offset,
+   sizeof(*qdev->ram_header));
+#else
  qdev->ram_header = ioremap(qdev->vram_base +
 qdev->rom->ram_header_offset,
 sizeof(*qdev->ram_header));
+#endif
  if (!qdev->ram_header) {
  DRM_ERROR("Unable to ioremap RAM header\n");
  r = -ENOMEM;



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: 回复: Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Christian König via Virtualization]

Hi Cong,

well than Dave must decide what to do here.

When QXL emulates a device it should also not use memory accesses which 
won't work on a physical device.


BTW: Your patch is really buggy, it misses the cases in ttm_module.c

Regards,
Christian.

Am 23.03.22 um 09:48 schrieb liuco...@kylinos.cn:


Hi Christian,


according to 'Arm Architecture Reference Manual Armv8,for Armv8-A

architecture profile' pdf E2.6.5


E2.6.5 Generation of Alignment faults by load/store multiple accesses to

 Device memory


When FEAT_LSMAOC is implemented and the value of the applicable nTLSMD

field is 0, any memory access by an AArch32 Load Multiple or Store

Multiple instruction to an address that the stage 1 translation

assigns as Device-nGRE, Device-nGnRE, or Device-nGnRnE generates

an Alignment fault.


so it seems not just some ARM boards doesn't allow unaligned access to 
MMIO


space, all pci memory mapped as Device-nGnRE in arm64 cannot support

unaligned access. and qxl is a device simulated by qemu, it should be 
able to access


to MMIO space in a more flexible way(PROT_NORMAL) than the real physical

graphics card.






Cong.





*主　题：*Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on 
arm64

*日　期：*2022-03-23 15:15
*发件人：*Christian König
*收件人：*Cong 
Liuairlied@redhat.comkraxel@redhat.comairlied@linux.iedaniel@ffwll.chray.huang@amd.comvirtualization@lists.linux-foundation.orgspice-devel@lists.freedesktop.orgdri-de...@lists.freedesktop.org 




Am 22.03.22 um 10:34 schrieb Cong Liu:
> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> the device is mapped as DEVICE_nGnRE, it can not support unaligned
> access.

Well that some ARM boards doesn't allow unaligned access to MMIO space
is a well known bug of those ARM boards.

So as far as I know this is a hardware bug you are trying to workaround
here and I'm not 100% sure that this is correct.

Christian.

>
>    6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
> [    6.620961] lr : setup_slot+0x34/0xf0 [qxl]
> [    6.621376] sp : 800012b73760
> [    6.621701] x29: 800012b73760 x28: 0001 x27: 
1000
> [    6.622400] x26: 0001 x25: 0400 x24: 
cf376848c000
> [    6.623099] x23: c4087400 x22: cf3718e17140 x21: 

> [    6.623823] x20: c4086000 x19: c40870b0 x18: 
0014
> [    6.624519] x17: 4d3605ab x16: bb3b6129 x15: 
6e771809
> [    6.625214] x14: 0001 x13: 007473696c5f7974 x12: 
696e69615f65
> [    6.625909] x11: d543656a x10:  x9 : 
cf3718e085a4
> [    6.626616] x8 : 006c7871 x7 : 000a x6 : 
0017
> [    6.627343] x5 : 1400 x4 : 800011f63400 x3 : 
1400
> [    6.628047] x2 :  x1 : c40870b0 x0 : 
c4086000

> [    6.628751] Call trace:
> [    6.628994]  setup_hw_slot+0x24/0x60 [qxl]
> [    6.629404]  setup_slot+0x34/0xf0 [qxl]
> [    6.629790]  qxl_device_init+0x6f0/0x7f0 [qxl]
> [    6.630235]  qxl_pci_probe+0xdc/0x1d0 [qxl]
> [    6.630654]  local_pci_probe+0x48/0xb8
> [    6.631027]  pci_device_probe+0x194/0x208
> [    6.631464]  really_probe+0xd0/0x458
> [    6.631818]  __driver_probe_device+0x124/0x1c0
> [    6.632256]  driver_probe_device+0x48/0x130
> [    6.632669]  __driver_attach+0xc4/0x1a8
> [    6.633049]  bus_for_each_dev+0x78/0xd0
> [    6.633437]  driver_attach+0x2c/0x38
> [    6.633789]  bus_add_driver+0x154/0x248
> [    6.634168]  driver_register+0x6c/0x128
> [    6.635205]  __pci_register_driver+0x4c/0x58
> [    6.635628]  qxl_init+0x48/0x1000 [qxl]
> [    6.636013]  do_one_initcall+0x50/0x240
> [    6.636390]  do_init_module+0x60/0x238
> [    6.636768]  load_module+0x2458/0x2900
> [    6.637136]  __do_sys_finit_module+0xbc/0x128
> [    6.637561]  __arm64_sys_finit_module+0x28/0x38
> [    6.638004]  invoke_syscall+0x74/0xf0
> [    6.638366]  el0_svc_common.constprop.0+0x58/0x1a8
> [    6.638836]  do_el0_svc+0x2c/0x90
> [    6.639216]  el0_svc+0x40/0x190
> [    6.639526]  el0t_64_sync_handler+0xb0/0xb8
> [    6.639934]  el0t_64_sync+0x1a4/0x1a8
> [    6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
> [    6.640889] ---[ end trace 95615d89b7c87f95 ]---
>
> Signed-off-by: Cong Liu
> ---
>   drivers/gpu/drm/qxl/qxl_kms.c | 10 ++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c 
b/drivers/gpu/drm/qxl/qxl_kms.c

> index 4dc5ad13f12c..0e61ac04d8ad 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
>   (int)qdev->surfaceram_size / 1024,
>   (sb == 4) ? "64bit" : "32bit");
>
> +#ifdef CONFIG_ARM64
> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
> +#else
>   qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
> +#endif
>   if (!qdev->rom) {
>   pr_err("Unable to ioremap ROM\n");

Re: [PATCH 2/2] Revert "virtio_pci: harden MSI-X interrupts"

[Jason Wang]

On Wed, Mar 23, 2022 at 5:05 PM Marc Zyngier  wrote:
>
> On Wed, 23 Mar 2022 03:15:24 +,
> Jason Wang  wrote:
> >
> > This reverts commit 9e35276a5344f74d4a3600fc4100b3dd251d5c56. Issue
> > were reported for the drivers that are using affinity managed IRQ
> > where manually toggling IRQ status is not expected. And we forget to
> > enable the interrupts in the restore path as well.
> >
> > In the future, we will rework on the interrupt hardening.
> >
> > Fixes: 9e35276a5344 ("virtio_pci: harden MSI-X interrupts")
> > Reported-by: Marc Zyngier 
> > Reported-by: Stefano Garzarella 
> > Signed-off-by: Jason Wang 
>
> For this patch and the INTx one:
>
> Acked-by: Marc Zyngier 
>
> Please keep me in the loop when you decide to rework this.

Ok.

Thanks

>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2] virtio: pci: sanity check bar indexes

[Jason Wang]

On Wed, Mar 23, 2022 at 5:13 PM Keir Fraser  wrote:
>
> On Wed, Mar 23, 2022 at 03:57:59PM +0800, Jason Wang wrote:
> > On Tue, Mar 22, 2022 at 11:20 PM Keir Fraser  wrote:
> > >
> > > The bar index is used as an index into the device's resource list
> > > and should be checked as within range for a standard bar.
> > >
> > > Also clean up an existing check to consistently use PCI_STD_NUM_BARS.
> > >
> > > Signed-off-by: Keir Fraser 
> > > ---
> > >  drivers/virtio/virtio_pci_modern.c | 10 --
> > >  drivers/virtio/virtio_pci_modern_dev.c |  8 +++-
> > >  2 files changed, 15 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_pci_modern.c 
> > > b/drivers/virtio/virtio_pci_modern.c
> > > index 5455bc041fb6..84bace98dff5 100644
> > > --- a/drivers/virtio/virtio_pci_modern.c
> > > +++ b/drivers/virtio/virtio_pci_modern.c
> > > @@ -293,7 +293,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > > *dev, u8 required_id,
> > >
> > > for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR); pos > 0;
> > >  pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> > > -   u8 type, cap_len, id;
> > > +   u8 type, cap_len, id, res_bar;
> > > u32 tmp32;
> > > u64 res_offset, res_length;
> > >
> > > @@ -317,7 +317,12 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > > *dev, u8 required_id,
> > >
> > > /* Type, and ID match, looks good */
> > > pci_read_config_byte(dev, pos + offsetof(struct 
> > > virtio_pci_cap,
> > > -bar), bar);
> > > +bar), _bar);
> > > +   if (res_bar >= PCI_STD_NUM_BARS) {
> > > +   dev_err(>dev, "%s: shm cap with bad bar: 
> > > %d\n",
> > > +   __func__, res_bar);
> > > +   continue;
> > > +   }
> > >
> > > /* Read the lower 32bit of length and offset */
> > > pci_read_config_dword(dev, pos + offsetof(struct 
> > > virtio_pci_cap,
> > > @@ -337,6 +342,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev 
> > > *dev, u8 required_id,
> > >  length_hi), );
> > > res_length |= ((u64)tmp32) << 32;
> > >
> > > +   *bar = res_bar;
> > > *offset = res_offset;
> > > *len = res_length;
> > >
> > > diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
> > > b/drivers/virtio/virtio_pci_modern_dev.c
> > > index e8b3ff2b9fbc..a6911d1e212a 100644
> > > --- a/drivers/virtio/virtio_pci_modern_dev.c
> > > +++ b/drivers/virtio/virtio_pci_modern_dev.c
> > > @@ -35,6 +35,12 @@ vp_modern_map_capability(struct 
> > > virtio_pci_modern_device *mdev, int off,
> > > pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, 
> > > length),
> > >   );
> > >
> > > +   if (bar >= PCI_STD_NUM_BARS) {
> > > +   dev_err(>dev,
> > > +   "virtio_pci: bad capability bar %u\n", bar);
> > > +   return NULL;
> > > +   }
> > > +
> > > if (length <= start) {
> > > dev_err(>dev,
> > > "virtio_pci: bad capability len %u (>%u 
> > > expected)\n",
> > > @@ -120,7 +126,7 @@ static inline int virtio_pci_find_capability(struct 
> > > pci_dev *dev, u8 cfg_type,
> > >  );
> > >
> > > /* Ignore structures with reserved BAR values */
> > > -   if (bar > 0x5)
> > > +   if (bar >= PCI_STD_NUM_BARS)
> > > continue;
> >
> > Just notice that the spec said:
> >
> > "
> > values 0x0 to 0x5 specify a Base Address register (BAR) belonging to
> > the function located beginning at 10h in PCI Configuration Space and
> > used to map the structure into Memory or I/O Space. The BAR is
> > permitted to be either 32-bit or 64-bit, it can map Memory Space or
> > I/O Space.
> >
> > Any other value is reserved for future use.
> > "
> >
> > So we probably need to stick 0x5 instead of 0x6 (PCI_STD_NUM_BARS) for
> > this and other places.
>
> The comparison(s) are changed to greater-or-equal, so they are logically
> equivalent to the old check against naked 0x5 while documenting the intent
> better.

You're right, So:

Acked-by: Jason Wang 

>
> > Thanks
> >
> > >
> > > if (type == cfg_type) {
> > > --
> > > 2.35.1.894.gb6a874cedc-goog
> > >
> >
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v2 3/3] vsock/virtio: read the negotiated features before using VQs

[Stefano Garzarella]

Complete the driver configuration, reading the negotiated features,
before using the VQs and tell the device that the driver is ready in
the virtio_vsock_probe().

Fixes: 53efbba12cc7 ("virtio/vsock: enable SEQPACKET for transport")
Suggested-by: Michael S. Tsirkin 
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index fff67ad39087..1244e7cf585b 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -622,6 +622,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
INIT_WORK(>event_work, virtio_transport_event_work);
INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
 
+   if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
+   vsock->seqpacket_allow = true;
+
vdev->priv = vsock;
virtio_device_ready(vdev);
 
@@ -639,9 +642,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
vsock->event_run = true;
mutex_unlock(>event_lock);
 
-   if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
-   vsock->seqpacket_allow = true;
-
rcu_assign_pointer(the_virtio_vsock, vsock);
 
mutex_unlock(_virtio_vsock_mutex);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v2 1/3] vsock/virtio: enable VQs early on probe

[Stefano Garzarella]

virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, but virtio-vsock
driver uses VQs in the probe function to fill rx and event VQs
with new buffers.

Let's fix this, calling virtio_device_ready() before using VQs
in the probe function.

Fixes: 0ea9e1d3a9e3 ("VSOCK: Introduce virtio_transport.ko")
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5afc194a58bb..b1962f8cd502 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -622,6 +622,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
INIT_WORK(>event_work, virtio_transport_event_work);
INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
 
+   virtio_device_ready(vdev);
+
mutex_lock(>tx_lock);
vsock->tx_run = true;
mutex_unlock(>tx_lock);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v2 2/3] vsock/virtio: initialize vdev->priv before using VQs

[Stefano Garzarella]

When we fill VQs with empty buffers and kick the host, it may send
an interrupt. `vdev->priv` must be initialized before this since it
is used in the virtqueue callback.

Fixes: 0deab087b16a ("vsock/virtio: use RCU to avoid use-after-free on 
the_virtio_vsock")
Suggested-by: Michael S. Tsirkin 
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index b1962f8cd502..fff67ad39087 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -622,6 +622,7 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
INIT_WORK(>event_work, virtio_transport_event_work);
INIT_WORK(>send_pkt_work, virtio_transport_send_pkt_work);
 
+   vdev->priv = vsock;
virtio_device_ready(vdev);
 
mutex_lock(>tx_lock);
@@ -641,7 +642,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
vsock->seqpacket_allow = true;
 
-   vdev->priv = vsock;
rcu_assign_pointer(the_virtio_vsock, vsock);
 
mutex_unlock(_virtio_vsock_mutex);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net v2 0/3] vsock/virtio: enable VQs early on probe and finish the setup before using them

[Stefano Garzarella]

The first patch fixes a virtio-spec violation. The other two patches
complete the driver configuration before using the VQs in the probe.

The patch order should simplify backporting in stable branches.

v2:
- patch 1 is not changed from v1
- added 2 patches to complete the driver configuration before using the
  VQs in the probe [MST]

v1: https://lore.kernel.org/netdev/20220322103823.83411-1-sgarz...@redhat.com/

Stefano Garzarella (3):
  vsock/virtio: enable VQs early on probe
  vsock/virtio: initialize vdev->priv before using VQs
  vsock/virtio: read the negotiated features before using VQs

 net/vmw_vsock/virtio_transport.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio: use virtio_device_ready() in virtio_device_restore()

[Jason Wang]

On Wed, Mar 23, 2022 at 4:04 PM Stefano Garzarella  wrote:
>
> On Wed, Mar 23, 2022 at 11:10:27AM +0800, Jason Wang wrote:
> >On Tue, Mar 22, 2022 at 10:07 PM Michael S. Tsirkin  wrote:
> >>
> >> On Tue, Mar 22, 2022 at 12:43:13PM +0100, Stefano Garzarella wrote:
> >> > After waking up a suspended VM, the kernel prints the following trace
> >> > for virtio drivers which do not directly call virtio_device_ready() in
> >> > the .restore:
> >> >
> >> > PM: suspend exit
> >> > irq 22: nobody cared (try booting with the "irqpoll" option)
> >> > Call Trace:
> >> >  
> >> >  dump_stack_lvl+0x38/0x49
> >> >  dump_stack+0x10/0x12
> >> >  __report_bad_irq+0x3a/0xaf
> >> >  note_interrupt.cold+0xb/0x60
> >> >  handle_irq_event+0x71/0x80
> >> >  handle_fasteoi_irq+0x95/0x1e0
> >> >  __common_interrupt+0x6b/0x110
> >> >  common_interrupt+0x63/0xe0
> >> >  asm_common_interrupt+0x1e/0x40
> >> >  ? __do_softirq+0x75/0x2f3
> >> >  irq_exit_rcu+0x93/0xe0
> >> >  sysvec_apic_timer_interrupt+0xac/0xd0
> >> >  
> >> >  
> >> >  asm_sysvec_apic_timer_interrupt+0x12/0x20
> >> >  arch_cpu_idle+0x12/0x20
> >> >  default_idle_call+0x39/0xf0
> >> >  do_idle+0x1b5/0x210
> >> >  cpu_startup_entry+0x20/0x30
> >> >  start_secondary+0xf3/0x100
> >> >  secondary_startup_64_no_verify+0xc3/0xcb
> >> >  
> >> > handlers:
> >> > [<8f9bac49>] vp_interrupt
> >> > [<8f9bac49>] vp_interrupt
> >> > Disabling IRQ #22
> >> >
> >> > This happens because we don't invoke .enable_cbs callback in
> >> > virtio_device_restore(). That callback is used by some transports
> >> > (e.g. virtio-pci) to enable interrupts.
> >> >
> >> > Let's fix it, by calling virtio_device_ready() as we do in
> >> > virtio_dev_probe(). This function calls .enable_cts callback and sets
> >> > DRIVER_OK status bit.
> >> >
> >> > This fix also avoids setting DRIVER_OK twice for those drivers that
> >> > call virtio_device_ready() in the .restore.
> >> >
> >> > Fixes: d50497eb4e55 ("virtio_config: introduce a new .enable_cbs method")
> >> > Signed-off-by: Stefano Garzarella 
> >> > ---
> >> >
> >> > I'm not sure about the fixes tag. That one is more generic, but the
> >> > following one I think introduced the issue.
> >> >
> >> > Fixes: 9e35276a5344 ("virtio_pci: harden MSI-X interrupts")
> >>
> >> Jason what should we do about this one BTW? Just revert? We have other
> >> issues ...
> >
> >Let me post a patch to revert it and give it a rework.
>
> Thanks for reverting it.
>
> Should we queue this patch anyway to prevent future issues and avoid
> setting DRIVER_OK twice?
>
> Please, let me know if I have to resend it by removing the call trace
> that after the revert should no longer occur.

I think we can have another version.

Thanks

>
> Thanks,
> Stefano
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio: use virtio_device_ready() in virtio_device_restore()

[Stefano Garzarella]

On Wed, Mar 23, 2022 at 11:10:27AM +0800, Jason Wang wrote:

On Tue, Mar 22, 2022 at 10:07 PM Michael S. Tsirkin  wrote:


On Tue, Mar 22, 2022 at 12:43:13PM +0100, Stefano Garzarella wrote:
> After waking up a suspended VM, the kernel prints the following trace
> for virtio drivers which do not directly call virtio_device_ready() in
> the .restore:
>
> PM: suspend exit
> irq 22: nobody cared (try booting with the "irqpoll" option)
> Call Trace:
>  
>  dump_stack_lvl+0x38/0x49
>  dump_stack+0x10/0x12
>  __report_bad_irq+0x3a/0xaf
>  note_interrupt.cold+0xb/0x60
>  handle_irq_event+0x71/0x80
>  handle_fasteoi_irq+0x95/0x1e0
>  __common_interrupt+0x6b/0x110
>  common_interrupt+0x63/0xe0
>  asm_common_interrupt+0x1e/0x40
>  ? __do_softirq+0x75/0x2f3
>  irq_exit_rcu+0x93/0xe0
>  sysvec_apic_timer_interrupt+0xac/0xd0
>  
>  
>  asm_sysvec_apic_timer_interrupt+0x12/0x20
>  arch_cpu_idle+0x12/0x20
>  default_idle_call+0x39/0xf0
>  do_idle+0x1b5/0x210
>  cpu_startup_entry+0x20/0x30
>  start_secondary+0xf3/0x100
>  secondary_startup_64_no_verify+0xc3/0xcb
>  
> handlers:
> [<8f9bac49>] vp_interrupt
> [<8f9bac49>] vp_interrupt
> Disabling IRQ #22
>
> This happens because we don't invoke .enable_cbs callback in
> virtio_device_restore(). That callback is used by some transports
> (e.g. virtio-pci) to enable interrupts.
>
> Let's fix it, by calling virtio_device_ready() as we do in
> virtio_dev_probe(). This function calls .enable_cts callback and sets
> DRIVER_OK status bit.
>
> This fix also avoids setting DRIVER_OK twice for those drivers that
> call virtio_device_ready() in the .restore.
>
> Fixes: d50497eb4e55 ("virtio_config: introduce a new .enable_cbs method")
> Signed-off-by: Stefano Garzarella 
> ---
>
> I'm not sure about the fixes tag. That one is more generic, but the
> following one I think introduced the issue.
>
> Fixes: 9e35276a5344 ("virtio_pci: harden MSI-X interrupts")

Jason what should we do about this one BTW? Just revert? We have other
issues ...


Let me post a patch to revert it and give it a rework.


Thanks for reverting it.

Should we queue this patch anyway to prevent future issues and avoid 
setting DRIVER_OK twice?


Please, let me know if I have to resend it by removing the call trace 
that after the revert should no longer occur.


Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2] virtio: pci: sanity check bar indexes

[Jason Wang]

On Tue, Mar 22, 2022 at 11:20 PM Keir Fraser  wrote:
>
> The bar index is used as an index into the device's resource list
> and should be checked as within range for a standard bar.
>
> Also clean up an existing check to consistently use PCI_STD_NUM_BARS.
>
> Signed-off-by: Keir Fraser 
> ---
>  drivers/virtio/virtio_pci_modern.c | 10 --
>  drivers/virtio/virtio_pci_modern_dev.c |  8 +++-
>  2 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern.c 
> b/drivers/virtio/virtio_pci_modern.c
> index 5455bc041fb6..84bace98dff5 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -293,7 +293,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, 
> u8 required_id,
>
> for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR); pos > 0;
>  pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> -   u8 type, cap_len, id;
> +   u8 type, cap_len, id, res_bar;
> u32 tmp32;
> u64 res_offset, res_length;
>
> @@ -317,7 +317,12 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, 
> u8 required_id,
>
> /* Type, and ID match, looks good */
> pci_read_config_byte(dev, pos + offsetof(struct 
> virtio_pci_cap,
> -bar), bar);
> +bar), _bar);
> +   if (res_bar >= PCI_STD_NUM_BARS) {
> +   dev_err(>dev, "%s: shm cap with bad bar: %d\n",
> +   __func__, res_bar);
> +   continue;
> +   }
>
> /* Read the lower 32bit of length and offset */
> pci_read_config_dword(dev, pos + offsetof(struct 
> virtio_pci_cap,
> @@ -337,6 +342,7 @@ static int virtio_pci_find_shm_cap(struct pci_dev *dev, 
> u8 required_id,
>  length_hi), );
> res_length |= ((u64)tmp32) << 32;
>
> +   *bar = res_bar;
> *offset = res_offset;
> *len = res_length;
>
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
> b/drivers/virtio/virtio_pci_modern_dev.c
> index e8b3ff2b9fbc..a6911d1e212a 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -35,6 +35,12 @@ vp_modern_map_capability(struct virtio_pci_modern_device 
> *mdev, int off,
> pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, 
> length),
>   );
>
> +   if (bar >= PCI_STD_NUM_BARS) {
> +   dev_err(>dev,
> +   "virtio_pci: bad capability bar %u\n", bar);
> +   return NULL;
> +   }
> +
> if (length <= start) {
> dev_err(>dev,
> "virtio_pci: bad capability len %u (>%u expected)\n",
> @@ -120,7 +126,7 @@ static inline int virtio_pci_find_capability(struct 
> pci_dev *dev, u8 cfg_type,
>  );
>
> /* Ignore structures with reserved BAR values */
> -   if (bar > 0x5)
> +   if (bar >= PCI_STD_NUM_BARS)
> continue;

Just notice that the spec said:

"
values 0x0 to 0x5 specify a Base Address register (BAR) belonging to
the function located beginning at 10h in PCI Configuration Space and
used to map the structure into Memory or I/O Space. The BAR is
permitted to be either 32-bit or 64-bit, it can map Memory Space or
I/O Space.

Any other value is reserved for future use.
"

So we probably need to stick 0x5 instead of 0x6 (PCI_STD_NUM_BARS) for
this and other places.

Thanks

>
> if (type == cfg_type) {
> --
> 2.35.1.894.gb6a874cedc-goog
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: Re: [PATCH v3 0/6] Support akcipher for virtio-crypto

[zhenwei pi]

On 3/23/22 13:17, Eric Biggers wrote:

On Wed, Mar 23, 2022 at 10:49:06AM +0800, zhenwei pi wrote:

v2 -> v3:
- Introduce akcipher types to qapi
- Add test/benchmark suite for akcipher class
- Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
   - crypto: Introduce akcipher crypto class
   - virtio-crypto: Introduce RSA algorithm

v1 -> v2:
- Update virtio_crypto.h from v2 version of related kernel patch.

v1:
- Support akcipher for virtio-crypto.
- Introduce akcipher class.
- Introduce ASN1 decoder into QEMU.
- Implement RSA backend by nettle/hogweed.

Lei He (3):
   crypto-akcipher: Introduce akcipher types to qapi
   crypto: Implement RSA algorithm by hogweed
   tests/crypto: Add test suite for crypto akcipher

Zhenwei Pi (3):
   virtio-crypto: header update
   crypto: Introduce akcipher crypto class
   virtio-crypto: Introduce RSA algorithm


You forgot to describe the point of this patchset and what its use case is.
Like any other Linux kernel patchset, that needs to be in the cover letter.

- Eric

Thanks Eric for pointing this missing part.

This feature provides akcipher service offloading capability. QEMU side 
handles asymmetric requests via virtio-crypto devices from guest side, 
do encrypt/decrypt/sign/verify operations on host side, and return the 
result to guest.


This patchset implements a RSA backend by hogweed from nettle, it works 
together with guest patch:

https://lkml.org/lkml/2022/3/1/1425

--
zhenwei pi
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Christian König via Virtualization]

Am 22.03.22 um 10:34 schrieb Cong Liu:

qxl use ioremap to map ram_header and rom, in the arm64 implementation,
the device is mapped as DEVICE_nGnRE, it can not support unaligned
access.


Well that some ARM boards doesn't allow unaligned access to MMIO space 
is a well known bug of those ARM boards.


So as far as I know this is a hardware bug you are trying to workaround 
here and I'm not 100% sure that this is correct.


Christian.



   6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
[6.620961] lr : setup_slot+0x34/0xf0 [qxl]
[6.621376] sp : 800012b73760
[6.621701] x29: 800012b73760 x28: 0001 x27: 1000
[6.622400] x26: 0001 x25: 0400 x24: cf376848c000
[6.623099] x23: c4087400 x22: cf3718e17140 x21: 
[6.623823] x20: c4086000 x19: c40870b0 x18: 0014
[6.624519] x17: 4d3605ab x16: bb3b6129 x15: 6e771809
[6.625214] x14: 0001 x13: 007473696c5f7974 x12: 696e69615f65
[6.625909] x11: d543656a x10:  x9 : cf3718e085a4
[6.626616] x8 : 006c7871 x7 : 000a x6 : 0017
[6.627343] x5 : 1400 x4 : 800011f63400 x3 : 1400
[6.628047] x2 :  x1 : c40870b0 x0 : c4086000
[6.628751] Call trace:
[6.628994]  setup_hw_slot+0x24/0x60 [qxl]
[6.629404]  setup_slot+0x34/0xf0 [qxl]
[6.629790]  qxl_device_init+0x6f0/0x7f0 [qxl]
[6.630235]  qxl_pci_probe+0xdc/0x1d0 [qxl]
[6.630654]  local_pci_probe+0x48/0xb8
[6.631027]  pci_device_probe+0x194/0x208
[6.631464]  really_probe+0xd0/0x458
[6.631818]  __driver_probe_device+0x124/0x1c0
[6.632256]  driver_probe_device+0x48/0x130
[6.632669]  __driver_attach+0xc4/0x1a8
[6.633049]  bus_for_each_dev+0x78/0xd0
[6.633437]  driver_attach+0x2c/0x38
[6.633789]  bus_add_driver+0x154/0x248
[6.634168]  driver_register+0x6c/0x128
[6.635205]  __pci_register_driver+0x4c/0x58
[6.635628]  qxl_init+0x48/0x1000 [qxl]
[6.636013]  do_one_initcall+0x50/0x240
[6.636390]  do_init_module+0x60/0x238
[6.636768]  load_module+0x2458/0x2900
[6.637136]  __do_sys_finit_module+0xbc/0x128
[6.637561]  __arm64_sys_finit_module+0x28/0x38
[6.638004]  invoke_syscall+0x74/0xf0
[6.638366]  el0_svc_common.constprop.0+0x58/0x1a8
[6.638836]  do_el0_svc+0x2c/0x90
[6.639216]  el0_svc+0x40/0x190
[6.639526]  el0t_64_sync_handler+0xb0/0xb8
[6.639934]  el0t_64_sync+0x1a4/0x1a8
[6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
[6.640889] ---[ end trace 95615d89b7c87f95 ]---

Signed-off-by: Cong Liu 
---
  drivers/gpu/drm/qxl/qxl_kms.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 4dc5ad13f12c..0e61ac04d8ad 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
 (int)qdev->surfaceram_size / 1024,
 (sb == 4) ? "64bit" : "32bit");
  
+#ifdef CONFIG_ARM64

+   qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
+#else
qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
+#endif
if (!qdev->rom) {
pr_err("Unable to ioremap ROM\n");
r = -ENOMEM;
@@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
goto rom_unmap;
}
  
+#ifdef CONFIG_ARM64

+   qdev->ram_header = ioremap_cache(qdev->vram_base +
+  qdev->rom->ram_header_offset,
+  sizeof(*qdev->ram_header));
+#else
qdev->ram_header = ioremap(qdev->vram_base +
   qdev->rom->ram_header_offset,
   sizeof(*qdev->ram_header));
+#endif
if (!qdev->ram_header) {
DRM_ERROR("Unable to ioremap RAM header\n");
r = -ENOMEM;


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization