From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 91BE41527B3 for ; Tue, 11 Jun 2024 22:03:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718143397; cv=none; b=DFI36DH+sAUGDqqsqwIDmSPxEBmWgw0tOC101e+q9ASZuFJ+ZsviEgeV/eBWqYVOEGCK1uqZUr+22x/HLWWQPPAZXcDeDi3H9aoTb9O1nQ7UqknxcJfQOT5aU2Etr5Ooq3iuLx7T3OdIjUFOBxkHXDFUpOHvmhp8baM1FEU4Z3Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718143397; c=relaxed/simple; bh=Ey4dpn+f1UTvaOx0gUOY+eQ9wwdUJpZk5SoYV20o088=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=eTp3KDeCZXHNwQtln6SieWNPQOLeEdcN4bmXws/b4fPw7hzqlqljUIYvxlMY5Fzdiiq+hlJZfk9z0izVKXzEBn/MUeDo10n+S+jKC0v1vTOvr0APlKP9D3JOqZYL56ZLgCXneZGLRq6w7kyk/RrtYYT75nX54iFTMZt1nAeXfCs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=pzfp13xJ; arc=none smtp.client-ip=209.85.128.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="pzfp13xJ" Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-62f46f56353so8373867b3.3 for ; Tue, 11 Jun 2024 15:03:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1718143393; x=1718748193; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=KeC1/70he5CQ79E8jYxK5W/LoMc72KfRPBd07uT+cuY=; b=pzfp13xJgS4PrMgr8eebFpdqon9XpjtuiiSunvsOv79NWb1mBwZL/k+sTbiz2FB6NB /X1X3LWw1TPj6w5qDUh8WVXnNvR265QOTBi18YzRyOplSSV0C/CVWB3SDsGm2u59XBeL CK/JgZVeh0faOENI/V7ZI/wdUQXf7YrJVciyplIgAnOisIgTsGYu1JUlalCTZZYAI4KY c/I+h4hb6zXubWue6dYhishxiqsCIr1PSTxdW6/cF16uBq8P5qZ+VAmHZ7bAzUClX4sN SvjLPvvxWPA5kPiFUvS1K33h2hBsePMcEqsdABwkE8HNsX3G5RPn9c4ThHVXPPw/M3i9 rOdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718143393; x=1718748193; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KeC1/70he5CQ79E8jYxK5W/LoMc72KfRPBd07uT+cuY=; b=OQzb6q7jjU7uOp35clGsCHyg4GAZf3Ilh7pGhj45Gk3Re85uQzdcU1zSNQhPdykaYK PJtIWEgt6TtkwErmCng6tzBFiyF+MtYIIb/xbifIhYsDIr7pVkG+UVeF4M55ZNzwHhT3 1lOcgN5WHIrOPpfWej7yWO5tjfaUb2fs6WXa/Cdhi5m4OGSW75DpAWavdGZNKH5+MzEm EuXohhLR5zWroeCb9vWKedIMWh3saE+F//QtSGbzxEuy/sZf3Eyx2Lj+Ja/E1HjWCmES 4riJoZ7bAUI3hWjUs8N5vUvkLUnXWqkCDMHiUWVjFKnz4WNoSvQSTuu34q8r2kdMlXt/ uKtA== X-Forwarded-Encrypted: i=1; AJvYcCX8g6XTvjX/0mzqtHaTJO5ArZsCYFRb61dOvWT7aws2iKqDue7URvwK2Qn0WGnekXE+0NWymItCHf43++UslDyPUIhOcMjXFXcC82mG X-Gm-Message-State: AOJu0Yy4/zOzHciNEUJ9e5wjo0IUPlT4QN9k5tHglYKpL4vx0j4vWI6N cc77/Nbf/lUmG3rrEEexKgc+Z4Io1a6zS2YBwpbVYY8sUKm4cHYGJVM9/nOmOVyEAV5icHk0bQ9 UhA== X-Google-Smtp-Source: AGHT+IEgj86esgclw1q8kzIIbHibvWuASiwr8am2UV+qPS22vwZD9KxmZ6peui4uE5upeZ1rOL+uSOW+Dn4= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a05:690c:c17:b0:627:a961:caee with SMTP id 00721157ae682-62fba27ded9mr55487b3.4.1718143393450; Tue, 11 Jun 2024 15:03:13 -0700 (PDT) Date: Tue, 11 Jun 2024 15:03:11 -0700 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: Message-ID: Subject: Re: [PATCH V8 1/2] KVM: selftests: Add x86_64 guest udelay() utility From: Sean Christopherson To: Reinette Chatre Cc: isaku.yamahata@intel.com, pbonzini@redhat.com, erdemaktas@google.com, vkuznets@redhat.com, vannapurve@google.com, jmattson@google.com, mlevitsk@redhat.com, xiaoyao.li@intel.com, chao.gao@intel.com, rick.p.edgecombe@intel.com, yuan.yao@intel.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Tue, Jun 11, 2024, Reinette Chatre wrote: > > Heh, the docs are stale. KVM hasn't returned an error since commit cc578287e322 > > ("KVM: Infrastructure for software and hardware based TSC rate scaling"), which > > again predates selftests by many years (6+ in this case). To make our lives > > much simpler, I think we should assert that KVM_GET_TSC_KHZ succeeds, and maybe > > throw in a GUEST_ASSERT(thz_khz) in udelay()? > > I added the GUEST_ASSERT() but I find that it comes with a caveat (more below). > > I plan an assert as below that would end up testing the same as what a > GUEST_ASSERT(tsc_khz) would accomplish: > > r = __vm_ioctl(vm, KVM_GET_TSC_KHZ, NULL); > TEST_ASSERT(r > 0, "KVM_GET_TSC_KHZ did not provide a valid TSC freq."); > tsc_khz = r; > > > Caveat is: the additional GUEST_ASSERT() requires all tests that use udelay() in > the guest to now subtly be required to implement a ucall (UCALL_ABORT) handler. > I did a crude grep check to see and of the 69 x86_64 tests there are 47 that do > indeed have a UCALL_ABORT handler. If any of the other use udelay() then the > GUEST_ASSERT() will of course still trigger, but will be quite cryptic. For > example, "Unhandled exception '0xe' at guest RIP '0x0'" vs. "tsc_khz". Yeah, we really need to add a bit more infrastructure, there is way, way, waaaay too much boilerplate needed just to run a guest and handle the basic ucalls. Reporting guest asserts should Just Work for 99.9% of tests. Anyways, is it any less cryptic if ucall_assert() forces a failure? I forget if the problem with an unhandled GUEST_ASSERT() is that the test re-enters the guest, or if it's something else. I don't think we need a perfect solution _now_, as tsc_khz really should never be 0, just something to not make life completely miserable for future developers. diff --git a/tools/testing/selftests/kvm/lib/ucall_common.c b/tools/testing/selftests/kvm/lib/ucall_common.c index 42151e571953..1116bce5cdbf 100644 --- a/tools/testing/selftests/kvm/lib/ucall_common.c +++ b/tools/testing/selftests/kvm/lib/ucall_common.c @@ -98,6 +98,8 @@ void ucall_assert(uint64_t cmd, const char *exp, const char *file, ucall_arch_do_ucall((vm_vaddr_t)uc->hva); + ucall_arch_do_ucall(GUEST_UCALL_FAILED); + ucall_free(uc); }