From nobody Mon Feb 9 11:29:38 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1630684787110335.03682548389554; Fri, 3 Sep 2021 08:59:47 -0700 (PDT) Received: from localhost ([::1]:39768 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mMBbW-0001P2-1i for importer@patchew.org; Fri, 03 Sep 2021 11:59:46 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58194) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mMBY6-0002Fd-0A for qemu-devel@nongnu.org; Fri, 03 Sep 2021 11:56:14 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:23958) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mMBY2-0000pE-SR for qemu-devel@nongnu.org; Fri, 03 Sep 2021 11:56:13 -0400 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-403-8d1BLc3VO9StlWgffEr5TQ-1; Fri, 03 Sep 2021 11:56:08 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D03156D4E0; Fri, 3 Sep 2021 15:56:06 +0000 (UTC) Received: from t480s.redhat.com (unknown [10.39.193.142]) by smtp.corp.redhat.com (Postfix) with ESMTP id B2F791B42C; Fri, 3 Sep 2021 15:56:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1630684569; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+h4ftnUtGQz+g6VkOrGUTUGMechF5A5IGRiTry9Z1mU=; b=JKUSZl9eRA9looLJytlj7dVvAWkZy8KYNzhUZI627V1HP+bK9Ea+taRxeEBSjXCGW/4V+8 vBCUElW74ifJh29PBILYCQC/FO4LYExWJ+rDRz0/casd6U1ulKMlHd2G0a3+FrOilfujPg yd9n0hU0FToA3pSPoUjqatrCpJXtnn8= X-MC-Unique: 8d1BLc3VO9StlWgffEr5TQ-1 From: David Hildenbrand To: qemu-devel@nongnu.org Subject: [PATCH v3 13/13] hw/s390x/s390-skeys: lazy storage key enablement under TCG Date: Fri, 3 Sep 2021 17:55:14 +0200 Message-Id: <20210903155514.44772-14-david@redhat.com> In-Reply-To: <20210903155514.44772-1-david@redhat.com> References: <20210903155514.44772-1-david@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=216.205.24.124; envelope-from=david@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -31 X-Spam_score: -3.2 X-Spam_bar: --- X-Spam_report: (-3.2 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.392, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Jason J . Herne" , Thomas Huth , Janosch Frank , David Hildenbrand , Cornelia Huck , Richard Henderson , Halil Pasic , Christian Borntraeger , qemu-s390x@nongnu.org, Claudio Imbrenda Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1630684787794100005 Content-Type: text/plain; charset="utf-8" Let's enable storage keys lazily under TCG, just as we do under KVM. Only fairly old Linux versions actually make use of storage keys, so it can be kind of wasteful to allocate quite some memory and track changes and references if nobody cares. We have to make sure to flush the TLB when enabling storage keys after the VM was already running: otherwise it might happen that we don't catch references or modifications afterwards. Add proper documentation to all callbacks. The kvm-unit-tests skey tests keeps on working with this change. Signed-off-by: David Hildenbrand Reviewed-by: Thomas Huth --- hw/s390x/s390-skeys.c | 65 ++++++++++++++++++++++++++------- include/hw/s390x/storage-keys.h | 63 ++++++++++++++++++++++++++++++++ target/s390x/mmu_helper.c | 8 ++++ target/s390x/tcg/mem_helper.c | 9 +++++ 4 files changed, 131 insertions(+), 14 deletions(-) diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c index 9e994a5582..5024faf411 100644 --- a/hw/s390x/s390-skeys.c +++ b/hw/s390x/s390-skeys.c @@ -191,18 +191,45 @@ out: fclose(f); } =20 -static void qemu_s390_skeys_init(Object *obj) +static bool qemu_s390_skeys_are_enabled(S390SKeysState *ss) { - QEMUS390SKeysState *skeys =3D QEMU_S390_SKEYS(obj); - MachineState *machine =3D MACHINE(qdev_get_machine()); + QEMUS390SKeysState *skeys =3D QEMU_S390_SKEYS(ss); =20 - skeys->key_count =3D machine->ram_size / TARGET_PAGE_SIZE; - skeys->keydata =3D g_malloc0(skeys->key_count); + /* Lockless check is sufficient. */ + return !!skeys->keydata; } =20 -static bool qemu_s390_skeys_are_enabled(S390SKeysState *ss) +static bool qemu_s390_enable_skeys(S390SKeysState *ss) { - return true; + QEMUS390SKeysState *skeys =3D QEMU_S390_SKEYS(ss); + static gsize initialized; + + if (likely(skeys->keydata)) { + return true; + } + + /* + * TODO: Modern Linux doesn't use storage keys unless running KVM gues= ts + * that use storage keys. Therefore, we keep it simple for now. + * + * 1) We should initialize to "referenced+changed" for an initial + * over-indication. Let's avoid touching megabytes of data for now = and + * assume that any sane user will issue a storage key instruction b= efore + * actually relying on this data. + * 2) Relying on ram_size and allocating a big array is ugly. We should + * allocate and manage storage key data per RAMBlock or optimally u= sing + * some sparse data structure. + * 3) We only ever have a single S390SKeysState, so relying on + * g_once_init_enter() is good enough. + */ + if (g_once_init_enter(&initialized)) { + MachineState *machine =3D MACHINE(qdev_get_machine()); + + skeys->key_count =3D machine->ram_size / TARGET_PAGE_SIZE; + skeys->keydata =3D g_malloc0(skeys->key_count); + g_once_init_leave(&initialized, 1); + } + return false; } =20 static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn, @@ -212,9 +239,10 @@ static int qemu_s390_skeys_set(S390SKeysState *ss, uin= t64_t start_gfn, int i; =20 /* Check for uint64 overflow and access beyond end of key data */ - if (start_gfn + count > skeydev->key_count || start_gfn + count < coun= t) { - error_report("Error: Setting storage keys for page beyond the end " - "of memory: gfn=3D%" PRIx64 " count=3D%" PRId64, + if (unlikely(!skeydev->keydata || start_gfn + count > skeydev->key_cou= nt || + start_gfn + count < count)) { + error_report("Error: Setting storage keys for pages with unallocat= ed " + "storage key memory: gfn=3D%" PRIx64 " count=3D%" PRI= d64, start_gfn, count); return -EINVAL; } @@ -232,9 +260,10 @@ static int qemu_s390_skeys_get(S390SKeysState *ss, uin= t64_t start_gfn, int i; =20 /* Check for uint64 overflow and access beyond end of key data */ - if (start_gfn + count > skeydev->key_count || start_gfn + count < coun= t) { - error_report("Error: Getting storage keys for page beyond the end " - "of memory: gfn=3D%" PRIx64 " count=3D%" PRId64, + if (unlikely(!skeydev->keydata || start_gfn + count > skeydev->key_cou= nt || + start_gfn + count < count)) { + error_report("Error: Getting storage keys for pages with unallocat= ed " + "storage key memory: gfn=3D%" PRIx64 " count=3D%" PRI= d64, start_gfn, count); return -EINVAL; } @@ -251,6 +280,7 @@ static void qemu_s390_skeys_class_init(ObjectClass *oc,= void *data) DeviceClass *dc =3D DEVICE_CLASS(oc); =20 skeyclass->skeys_are_enabled =3D qemu_s390_skeys_are_enabled; + skeyclass->enable_skeys =3D qemu_s390_enable_skeys; skeyclass->get_skeys =3D qemu_s390_skeys_get; skeyclass->set_skeys =3D qemu_s390_skeys_set; =20 @@ -261,7 +291,6 @@ static void qemu_s390_skeys_class_init(ObjectClass *oc,= void *data) static const TypeInfo qemu_s390_skeys_info =3D { .name =3D TYPE_QEMU_S390_SKEYS, .parent =3D TYPE_S390_SKEYS, - .instance_init =3D qemu_s390_skeys_init, .instance_size =3D sizeof(QEMUS390SKeysState), .class_init =3D qemu_s390_skeys_class_init, .class_size =3D sizeof(S390SKeysClass), @@ -341,6 +370,14 @@ static int s390_storage_keys_load(QEMUFile *f, void *o= paque, int version_id) S390SKeysClass *skeyclass =3D S390_SKEYS_GET_CLASS(ss); int ret =3D 0; =20 + /* + * Make sure to lazy-enable if required to be done explicitly. No need= to + * flush any TLB as the VM is not running yet. + */ + if (skeyclass->enable_skeys) { + skeyclass->enable_skeys(ss); + } + while (!ret) { ram_addr_t addr; int flags; diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-key= s.h index eb091842c8..aa2ec2aae5 100644 --- a/include/hw/s390x/storage-keys.h +++ b/include/hw/s390x/storage-keys.h @@ -28,9 +28,72 @@ struct S390SKeysState { =20 struct S390SKeysClass { DeviceClass parent_class; + + /** + * @skeys_are_enabled: + * + * Check whether storage keys are enabled. If not enabled, they were n= ot + * enabled lazily either by the guest via a storage key instruction or + * by the host during migration. + * + * If disabled, everything not explicitly triggered by the guest, + * such as outgoing migration or dirty/change tracking, should not tou= ch + * storage keys and should not lazily enable it. + * + * @ks: the #S390SKeysState + * + * Returns false if not enabled and true if enabled. + */ bool (*skeys_are_enabled)(S390SKeysState *ks); + + /** + * @enable_skeys: + * + * Lazily enable storage keys. If this function is not implemented, + * setting a storage key will lazily enable storage keys implicitly + * instead. TCG guests have to make sure to flush the TLB of all CPUs + * if storage keys were not enabled before this call. + * + * @ks: the #S390SKeysState + * + * Returns false if not enabled before this call, and true if already + * enabled. + */ + bool (*enable_skeys)(S390SKeysState *ks); + + /** + * @get_skeys: + * + * Get storage keys for the given PFN range. This call will fail if + * storage keys have not been lazily enabled yet. + * + * Callers have to validate that a GFN is valid before this call. + * + * @ks: the #S390SKeysState + * @start_gfn: the start GFN to get storage keys for + * @count: the number of storage keys to get + * @keys: the byte array where storage keys will be stored to + * + * Returns 0 on success, returns an error if getting a storage key fai= led. + */ int (*get_skeys)(S390SKeysState *ks, uint64_t start_gfn, uint64_t coun= t, uint8_t *keys); + /** + * @set_skeys: + * + * Set storage keys for the given PFN range. This call will fail if + * storage keys have not been lazily enabled yet and implicit + * enablement is not supported. + * + * Callers have to validate that a GFN is valid before this call. + * + * @ks: the #S390SKeysState + * @start_gfn: the start GFN to set storage keys for + * @count: the number of storage keys to set + * @keys: the byte array where storage keys will be read from + * + * Returns 0 on success, returns an error if setting a storage key fai= led. + */ int (*set_skeys)(S390SKeysState *ks, uint64_t start_gfn, uint64_t coun= t, uint8_t *keys); }; diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c index e2b372efd9..b04b57c235 100644 --- a/target/s390x/mmu_helper.c +++ b/target/s390x/mmu_helper.c @@ -313,6 +313,14 @@ static void mmu_handle_skey(target_ulong addr, int rw,= int *flags) skeyclass =3D S390_SKEYS_GET_CLASS(ss); } =20 + /* + * Don't enable storage keys if they are still disabled, i.e., no actu= al + * storage key instruction was issued yet. + */ + if (!skeyclass->skeys_are_enabled(ss)) { + return; + } + /* * Whenever we create a new TLB entry, we set the storage key reference * bit. In case we allow write accesses, we set the storage key change diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c index 4f9f3e1f63..0bf775a37d 100644 --- a/target/s390x/tcg/mem_helper.c +++ b/target/s390x/tcg/mem_helper.c @@ -2186,6 +2186,9 @@ uint64_t HELPER(iske)(CPUS390XState *env, uint64_t r2) if (unlikely(!ss)) { ss =3D s390_get_skeys_device(); skeyclass =3D S390_SKEYS_GET_CLASS(ss); + if (skeyclass->enable_skeys && !skeyclass->enable_skeys(ss)) { + tlb_flush_all_cpus_synced(env_cpu(env)); + } } =20 rc =3D skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key); @@ -2213,6 +2216,9 @@ void HELPER(sske)(CPUS390XState *env, uint64_t r1, ui= nt64_t r2) if (unlikely(!ss)) { ss =3D s390_get_skeys_device(); skeyclass =3D S390_SKEYS_GET_CLASS(ss); + if (skeyclass->enable_skeys && !skeyclass->enable_skeys(ss)) { + tlb_flush_all_cpus_synced(env_cpu(env)); + } } =20 key =3D r1 & 0xfe; @@ -2244,6 +2250,9 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2) if (unlikely(!ss)) { ss =3D s390_get_skeys_device(); skeyclass =3D S390_SKEYS_GET_CLASS(ss); + if (skeyclass->enable_skeys && !skeyclass->enable_skeys(ss)) { + tlb_flush_all_cpus_synced(env_cpu(env)); + } } =20 rc =3D skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key); --=20 2.31.1