Re: [PATCH v1 1/2] s390/kvm: split kvm_s390_real_to_abs
On 19/03/2021 20.33, Claudio Imbrenda wrote: A new function _kvm_s390_real_to_abs will apply prefixing to a real address with a given prefix value. The old kvm_s390_real_to_abs becomes now a wrapper around the new function. This is needed to avoid code duplication in vSIE. Cc: sta...@vger.kernel.org Signed-off-by: Claudio Imbrenda --- arch/s390/kvm/gaccess.h | 23 +-- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h index daba10f76936..7c72a5e3449f 100644 --- a/arch/s390/kvm/gaccess.h +++ b/arch/s390/kvm/gaccess.h @@ -18,17 +18,14 @@ /** * kvm_s390_real_to_abs - convert guest real address to guest absolute address - * @vcpu - guest virtual cpu + * @prefix - guest prefix * @gra - guest real address * * Returns the guest absolute address that corresponds to the passed guest real - * address @gra of a virtual guest cpu by applying its prefix. + * address @gra of by applying the given prefix. */ -static inline unsigned long kvm_s390_real_to_abs(struct kvm_vcpu *vcpu, -unsigned long gra) +static inline unsigned long _kvm_s390_real_to_abs(u32 prefix, unsigned long gra) Just a matter of taste, but maybe this could be named differently? kvm_s390_real2abs_prefix() ? kvm_s390_prefix_real_to_abs()? Anyway: Reviewed-by: Thomas Huth
Re: [kvm-unit-tests PATCH] x86: Add tests for PKS
On 05/11/2020 09.18, Chenyi Qiang wrote:
This unit-test is intended to test the KVM support for Protection Keys
for Supervisor Pages (PKS). If CR4.PKS is set in long mode, supervisor
pkeys are checked in addition to normal paging protections and Access or
Write can be disabled via a MSR update without TLB flushes when
permissions change.
Signed-off-by: Chenyi Qiang
---
lib/x86/msr.h | 1 +
lib/x86/processor.h | 2 +
x86/Makefile.x86_64 | 1 +
x86/pks.c | 146
x86/unittests.cfg | 5 ++
5 files changed, 155 insertions(+)
create mode 100644 x86/pks.c
diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 6ef5502..e36934b 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -209,6 +209,7 @@
#define MSR_IA32_EBL_CR_POWERON 0x002a
#define MSR_IA32_FEATURE_CONTROL0x003a
#define MSR_IA32_TSC_ADJUST 0x003b
+#define MSR_IA32_PKRS 0x06e1
#define FEATURE_CONTROL_LOCKED(1<<0)
#define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX (1<<1)
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 74a2498..985fdd0 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -50,6 +50,7 @@
#define X86_CR4_SMEP 0x0010
#define X86_CR4_SMAP 0x0020
#define X86_CR4_PKE0x0040
+#define X86_CR4_PKS0x0100
#define X86_EFLAGS_CF0x0001
#define X86_EFLAGS_FIXED 0x0002
@@ -157,6 +158,7 @@ static inline u8 cpuid_maxphyaddr(void)
#define X86_FEATURE_RDPID (CPUID(0x7, 0, ECX, 22))
#define X86_FEATURE_SPEC_CTRL (CPUID(0x7, 0, EDX, 26))
#define X86_FEATURE_ARCH_CAPABILITIES (CPUID(0x7, 0, EDX, 29))
+#defineX86_FEATURE_PKS (CPUID(0x7, 0, ECX, 31))
#define X86_FEATURE_NX (CPUID(0x8001, 0, EDX, 20))
#define X86_FEATURE_RDPRU (CPUID(0x8008, 0, EBX, 4))
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index af61d85..3a353df 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -20,6 +20,7 @@ tests += $(TEST_DIR)/tscdeadline_latency.flat
tests += $(TEST_DIR)/intel-iommu.flat
tests += $(TEST_DIR)/vmware_backdoors.flat
tests += $(TEST_DIR)/rdpru.flat
+tests += $(TEST_DIR)/pks.flat
include $(SRCDIR)/$(TEST_DIR)/Makefile.common
diff --git a/x86/pks.c b/x86/pks.c
new file mode 100644
index 000..a3044cf
--- /dev/null
+++ b/x86/pks.c
@@ -0,0 +1,146 @@
+#include "libcflat.h"
+#include "x86/desc.h"
+#include "x86/processor.h"
+#include "x86/vm.h"
+#include "x86/msr.h"
+
+#define CR0_WP_MASK (1UL << 16)
+#define PTE_PKEY_BIT 59
+#define SUPER_BASE(1 << 24)
+#define SUPER_VAR(v) (*((__typeof__(&(v))) (((unsigned long)) +
SUPER_BASE)))
+
+volatile int pf_count = 0;
+volatile unsigned save;
+volatile unsigned test;
+
+static void set_cr0_wp(int wp)
+{
+unsigned long cr0 = read_cr0();
+
+cr0 &= ~CR0_WP_MASK;
+if (wp)
+cr0 |= CR0_WP_MASK;
+write_cr0(cr0);
+}
+
+void do_pf_tss(unsigned long error_code);
+void do_pf_tss(unsigned long error_code)
+{
+printf("#PF handler, error code: 0x%lx\n", error_code);
+pf_count++;
+save = test;
+wrmsr(MSR_IA32_PKRS, 0);
+}
+
+extern void pf_tss(void);
+
+asm ("pf_tss: \n\t"
+#ifdef __x86_64__
+// no task on x86_64, save/restore caller-save regs
+"push %rax; push %rcx; push %rdx; push %rsi; push %rdi\n"
+"push %r8; push %r9; push %r10; push %r11\n"
+"mov 9*8(%rsp), %rdi\n"
+#endif
+"call do_pf_tss \n\t"
+#ifdef __x86_64__
+"pop %r11; pop %r10; pop %r9; pop %r8\n"
+"pop %rdi; pop %rsi; pop %rdx; pop %rcx; pop %rax\n"
+#endif
+"add $"S", %"R "sp\n\t" // discard error code
+"iret"W" \n\t"
+"jmp pf_tss\n\t"
+);
+
+static void init_test(void)
+{
+pf_count = 0;
+
+invlpg();
+invlpg(_VAR(test));
+wrmsr(MSR_IA32_PKRS, 0);
+set_cr0_wp(0);
+}
+
+int main(int ac, char **av)
+{
+unsigned long i;
+unsigned int pkey = 0x2;
+unsigned int pkrs_ad = 0x10;
+unsigned int pkrs_wd = 0x20;
+
+if (!this_cpu_has(X86_FEATURE_PKS)) {
+printf("PKS not enabled\n");
+return report_summary();
+}
+
+setup_vm();
+setup_alt_stack();
+set_intr_alt_stack(14, pf_tss);
+wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_LMA);
+
+// First 16MB are user pages
+for (i = 0; i < SUPER_BASE; i += PAGE_SIZE) {
+*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) |= ((unsigned long)pkey
<< PTE_PKEY_BIT);
+invlpg((void *)i);
+}
+
+// Present the same 16MB as supervisor pages in the 16MB-32MB range
+for (i = SUPER_BASE; i < 2 * SUPER_BASE; i += PAGE_SIZE) {
+*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~SUPER_BASE;
+*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PT_USER_MASK;
+*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) |= ((unsigned long)pkey
Re: [PATCH 3/6] KVM: selftests: Convert iterations to int in dirty_log_perf_test
On 12/01/2021 22.42, Ben Gardon wrote: In order to add an iteration -1 to indicate that the memory population phase has not yet completed, convert the interations counters to ints. No functional change intended. Reviewed-by: Jacob Xu Signed-off-by: Ben Gardon --- .../selftests/kvm/dirty_log_perf_test.c | 26 +-- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c index 15a9c45bdb5f..3875f22d7283 100644 --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c @@ -28,8 +28,8 @@ static uint64_t guest_percpu_mem_size = DEFAULT_PER_VCPU_MEM_SIZE; /* Host variables */ static u64 dirty_log_manual_caps; static bool host_quit; -static uint64_t iteration; -static uint64_t vcpu_last_completed_iteration[KVM_MAX_VCPUS]; +static int iteration; +static int vcpu_last_completed_iteration[KVM_MAX_VCPUS]; Wouldn't it be better to use signed 64-bit variables instead? I.e. "int64_t" ? Thomas
Re: [PATCH 2/6] KVM: selftests: Avoid flooding debug log while populating memory
On 12/01/2021 22.42, Ben Gardon wrote:
Peter Xu pointed out that a log message printed while waiting for the
memory population phase of the dirty_log_perf_test will flood the debug
logs as there is no delay after printing the message. Since the message
does not provide much value anyway, remove it.
Reviewed-by: Jacob Xu
Signed-off-by: Ben Gardon
---
tools/testing/selftests/kvm/dirty_log_perf_test.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c
b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 16efe6589b43..15a9c45bdb5f 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -146,8 +146,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
/* Allow the vCPU to populate memory */
pr_debug("Starting iteration %lu - Populating\n", iteration);
while (READ_ONCE(vcpu_last_completed_iteration[vcpu_id]) != iteration)
- pr_debug("Waiting for vcpu_last_completed_iteration == %lu\n",
- iteration);
+ ;
ts_diff = timespec_elapsed(start);
pr_info("Populate memory time: %ld.%.9lds\n",
@@ -171,9 +170,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
pr_debug("Starting iteration %lu\n", iteration);
for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++) {
- while
(READ_ONCE(vcpu_last_completed_iteration[vcpu_id]) != iteration)
- pr_debug("Waiting for vCPU %d
vcpu_last_completed_iteration == %lu\n",
-vcpu_id, iteration);
+ while (READ_ONCE(vcpu_last_completed_iteration[vcpu_id])
+ != iteration)
+ ;
}
ts_diff = timespec_elapsed(start);
Reviewed-by: Thomas Huth
Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
On 29/05/2020 09.43, Like Xu wrote:
> When the full-width writes capability is set, use the alternative MSR
> range to write larger sign counter values (up to GP counter width).
>
> Signed-off-by: Like Xu
> ---
> lib/x86/msr.h | 1 +
> x86/pmu.c | 125 --
> 2 files changed, 102 insertions(+), 24 deletions(-)
[...]
> @@ -452,6 +468,66 @@ static void check_running_counter_wrmsr(void)
> report_prefix_pop();
> }
>
> +static void check_counters(void)
> +{
> + check_gp_counters();
> + check_fixed_counters();
> + check_rdpmc();
> + check_counters_many();
> + check_counter_overflow();
> + check_gp_counter_cmask();
> + check_running_counter_wrmsr();
> +}
> +
> +static void do_unsupported_width_counter_write(void *index)
> +{
> + wrmsr(MSR_IA32_PMC0 + *((int *) index), 0xff0123456789ull);
> +}
> +
> +static void check_gp_counters_write_width(void)
> +{
> + u64 val_64 = 0xff0123456789ull;
> + u64 val_32 = val_64 & ((1ul << 32) - 1);
Hi,
this broke compilation on 32-bit hosts:
https://travis-ci.com/github/huth/kvm-unit-tests/jobs/349654654#L710
Fix should be easy, I guess - either use 1ull or specify the mask
0x directly.
Thomas
Re: [PATCH trivial] KVM: PPC: Remove superfluous check for non-zero return value
On 18/09/2019 22.54, Greg Kurz wrote:
> On Wed, 18 Sep 2019 18:44:36 +0200
> Greg Kurz wrote:
>
>> On Wed, 11 Sep 2019 21:52:35 +0200
>> Thomas Huth wrote:
>>
>>> After the kfree()s haven been removed in the previous
>>> commit 9798f4ea71ea ("fix rollback when kvmppc_xive_create fails"),
>>> the code can be simplified even more to simply always "return ret"
>>> now.
>>>
>>> Signed-off-by: Thomas Huth
>>> ---
>>
>> This looks like a good candidate for trivial, hence Cc'ing Jiri
>> and adding trivial keyword in subject.
>>
>> Reviewed-by: Greg Kurz
>>
>
> Oops, the patch is correct but there are some fixes that require
> the return 0 to stay around...
>
> https://patchwork.ozlabs.org/project/kvm-ppc/list/?series=129957
:-)
Ok, then please simply ignore my patch.
Thomas
Re: [PATCH] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
On 13/09/2019 09.37, David Hildenbrand wrote: > On 13.09.19 09:34, Thomas Huth wrote: >> On 13/09/2019 09.20, Cornelia Huck wrote: >>> On Thu, 12 Sep 2019 13:23:38 +0200 >>> Thomas Huth wrote: >>> >>>> Hmm, we already talked about deprecating support for pre-3.15 kernel >>>> stuff in the past (see >>>> https://wiki.qemu.org/ChangeLog/2.12#Future_incompatible_changes for >>>> example), >>> >>> Btw: did we ever do that? I don't quite recall what code we were >>> talking about... >> >> We never really did - but we also never fixed the issue: If you run the >> current QEMU on a kernel before 3.15, it refuses to work due to the >> missing in-kernel FLIC device: >> >> Initialization of device s390-flic-kvm failed: KVM is missing capability >> KVM_CAP_DEVICE_CTR >> >> Since nobody really complained so far that running QEMU with KVM is >> still required on a kernel < 3.15, I think we could make this also >> "official" now and improve the error message a little bit, pointing the >> user to a kernel >= 3.15. > > Didn't we discuss back then to clean up *QEMU* and not the *kernel*? > Especially, to wait with cleanups until somebody requests to fix > instead. I mean you could have any user space in the wild that still > makes use of these interfaces ... Yes, that error message is part of QEMU, so I was referring to that one. Sorry for mixing this into a mail thread on the kernel mailing list :-/ Thomas
Re: [PATCH] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
On 13/09/2019 09.20, Cornelia Huck wrote: > On Thu, 12 Sep 2019 13:23:38 +0200 > Thomas Huth wrote: > >> Hmm, we already talked about deprecating support for pre-3.15 kernel >> stuff in the past (see >> https://wiki.qemu.org/ChangeLog/2.12#Future_incompatible_changes for >> example), > > Btw: did we ever do that? I don't quite recall what code we were > talking about... We never really did - but we also never fixed the issue: If you run the current QEMU on a kernel before 3.15, it refuses to work due to the missing in-kernel FLIC device: Initialization of device s390-flic-kvm failed: KVM is missing capability KVM_CAP_DEVICE_CTR Since nobody really complained so far that running QEMU with KVM is still required on a kernel < 3.15, I think we could make this also "official" now and improve the error message a little bit, pointing the user to a kernel >= 3.15. Thomas
[PATCH v2] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
When the userspace program runs the KVM_S390_INTERRUPT ioctl to inject
an interrupt, we convert them from the legacy struct kvm_s390_interrupt
to the new struct kvm_s390_irq via the s390int_to_s390irq() function.
However, this function does not take care of all types of interrupts
that we can inject into the guest later (see do_inject_vcpu()). Since we
do not clear out the s390irq values before calling s390int_to_s390irq(),
there is a chance that we copy random data from the kernel stack which
could be leaked to the userspace later.
Specifically, the problem exists with the KVM_S390_INT_PFAULT_INIT
interrupt: s390int_to_s390irq() does not handle it, and the function
__inject_pfault_init() later copies irq->u.ext which contains the
random kernel stack data. This data can then be leaked either to
the guest memory in __deliver_pfault_init(), or the userspace might
retrieve it directly with the KVM_S390_GET_IRQ_STATE ioctl.
Fix it by handling that interrupt type in s390int_to_s390irq(), too,
and by making sure that the s390irq struct is properly pre-initialized.
And while we're at it, make sure that s390int_to_s390irq() now
directly returns -EINVAL for unknown interrupt types, so that we
immediately get a proper error code in case we add more interrupt
types to do_inject_vcpu() without updating s390int_to_s390irq()
sometime in the future.
Cc: sta...@vger.kernel.org
Reviewed-by: David Hildenbrand
Reviewed-by: Christian Borntraeger
Signed-off-by: Thomas Huth
---
arch/s390/kvm/interrupt.c | 10 ++
arch/s390/kvm/kvm-s390.c | 2 +-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 3e7efdd9228a..165dea4c7f19 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1960,6 +1960,16 @@ int s390int_to_s390irq(struct kvm_s390_interrupt
*s390int,
case KVM_S390_MCHK:
irq->u.mchk.mcic = s390int->parm64;
break;
+ case KVM_S390_INT_PFAULT_INIT:
+ irq->u.ext.ext_params = s390int->parm;
+ irq->u.ext.ext_params2 = s390int->parm64;
+ break;
+ case KVM_S390_RESTART:
+ case KVM_S390_INT_CLOCK_COMP:
+ case KVM_S390_INT_CPU_TIMER:
+ break;
+ default:
+ return -EINVAL;
}
return 0;
}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index f329dcb3f44c..082eac2abc88 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -4323,7 +4323,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
}
case KVM_S390_INTERRUPT: {
struct kvm_s390_interrupt s390int;
- struct kvm_s390_irq s390irq;
+ struct kvm_s390_irq s390irq = {};
if (copy_from_user(, argp, sizeof(s390int)))
return -EFAULT;
--
2.18.1
Re: [PATCH] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
On 12/09/2019 12.52, Christian Borntraeger wrote:
>
>
> On 12.09.19 11:20, Thomas Huth wrote:
>> On 12/09/2019 11.14, David Hildenbrand wrote:
>>> On 12.09.19 11:00, Thomas Huth wrote:
>>>> When the userspace program runs the KVM_S390_INTERRUPT ioctl to inject
>>>> an interrupt, we convert them from the legacy struct kvm_s390_interrupt
>>>> to the new struct kvm_s390_irq via the s390int_to_s390irq() function.
>>>> However, this function does not take care of all types of interrupts
>>>> that we can inject into the guest later (see do_inject_vcpu()). Since we
>>>> do not clear out the s390irq values before calling s390int_to_s390irq(),
>>>> there is a chance that we copy unwanted data from the kernel stack
>>>> into the guest memory later if the interrupt data has not been properly
>>>> initialized by s390int_to_s390irq().
>>>>
>>>> Specifically, the problem exists with the KVM_S390_INT_PFAULT_INIT
>>>> interrupt: s390int_to_s390irq() does not handle it, but the function
>>>> __deliver_pfault_init() will later copy the uninitialized stack data
>>>> from the ext.ext_params2 into the guest memory.
>>>>
>>>> Fix it by handling that interrupt type in s390int_to_s390irq(), too.
>>>> And while we're at it, make sure that s390int_to_s390irq() now
>>>> directly returns -EINVAL for unknown interrupt types, so that we
>>>> do not run into this problem again in case we add more interrupt
>>>> types to do_inject_vcpu() sometime in the future.
>>>>
>>>> Signed-off-by: Thomas Huth
>>>> ---
>>>> arch/s390/kvm/interrupt.c | 10 ++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
>>>> index 3e7efdd9228a..165dea4c7f19 100644
>>>> --- a/arch/s390/kvm/interrupt.c
>>>> +++ b/arch/s390/kvm/interrupt.c
>>>> @@ -1960,6 +1960,16 @@ int s390int_to_s390irq(struct kvm_s390_interrupt
>>>> *s390int,
>>>>case KVM_S390_MCHK:
>>>>irq->u.mchk.mcic = s390int->parm64;
>>>>break;
>>>> + case KVM_S390_INT_PFAULT_INIT:
>>>> + irq->u.ext.ext_params = s390int->parm;
>>>> + irq->u.ext.ext_params2 = s390int->parm64;
>>>> + break;
>>>> + case KVM_S390_RESTART:
>>>> + case KVM_S390_INT_CLOCK_COMP:
>>>> + case KVM_S390_INT_CPU_TIMER:
>>>> + break;
>>>> + default:
>>>> + return -EINVAL;
>>>>}
>>>>return 0;
>>>> }
>>>>
>>>
>>> Wouldn't a safe fix be to initialize the struct to zero in the caller?
>>
>> That's of course possible, too. But that means that we always have to
>> zero out the whole structure, so that's a little bit more of overhead
>> (well, it likely doesn't matter for such a legacy ioctl).
>
> Yes doing something like
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index c19a24e940a1..b1f6f434af5d 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -4332,7 +4332,7 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
> }
> case KVM_S390_INTERRUPT: {
> struct kvm_s390_interrupt s390int;
> - struct kvm_s390_irq s390irq;
> + struct kvm_s390_irq s390irq = {};
>
> if (copy_from_user(, argp, sizeof(s390int)))
> return -EFAULT;
>
> would certainly be ok as well, but
I don't think that it's urgently necessary, but ok, if you all prefer to
have it, too, I can add it to my patch.
>> But the more important question: Do we then still care of fixing the
>> PFAULT_INIT interrupt here? Since it requires a parameter, the "case
>> KVM_S390_INT_PFAULT_INIT:" part would be required here anyway.
>
> as long as we we this interface we should fix it and we should do the
> pfault thing correctly.
> Maybe we should start to deprecate this interface and remove it.
Hmm, we already talked about deprecating support for pre-3.15 kernel
stuff in the past (see
https://wiki.qemu.org/ChangeLog/2.12#Future_incompatible_changes for
example), since this has been broken in QEMU since quite a while, but
the new KVM_S390_IRQ replacement has just been introduced with kernel
4.1 ... so removing this KVM_S390_INTERRUPT ioctl any time soon sounds
wrong to me, we might break some userspace programs that are still there
in the wild...
Thomas
Re: [PATCH] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
On 12/09/2019 12.47, Christian Borntraeger wrote: > > > On 12.09.19 11:00, Thomas Huth wrote: >> When the userspace program runs the KVM_S390_INTERRUPT ioctl to inject >> an interrupt, we convert them from the legacy struct kvm_s390_interrupt >> to the new struct kvm_s390_irq via the s390int_to_s390irq() function. >> However, this function does not take care of all types of interrupts >> that we can inject into the guest later (see do_inject_vcpu()). Since we >> do not clear out the s390irq values before calling s390int_to_s390irq(), >> there is a chance that we copy unwanted data from the kernel stack >> into the guest memory later if the interrupt data has not been properly >> initialized by s390int_to_s390irq(). > > You mean by using the migration callbacks to get all interrupts back to > userspace? Oh, I was not thinking about GET_IRQ_STATE yet, I was thinking about __deliver_pfault_init() which would deliver the value into the guest memory (from where the userspace program could extract it again). >> Specifically, the problem exists with the KVM_S390_INT_PFAULT_INIT >> interrupt: s390int_to_s390irq() does not handle it, but the function >> __deliver_pfault_init() will later copy the uninitialized stack data >> from the ext.ext_params2 into the guest memory. > > Shouldnt we add some more detailed description how this can happen? > Something like > "By using the KVM_S390_INTERRUPT ioctl with a KVM_S390_INT_PFAULT_INIT > interrupt followed by the KVM_S390_GET_IRQ_STATE ioctl the user can > extract a value from the kernel stack." GET_IRQ_STATE certainly deserves to be mentioned here, I'll add it to the patch description and will send a v2. Thomas
Re: [PATCH] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
On 12/09/2019 11.14, David Hildenbrand wrote: > On 12.09.19 11:00, Thomas Huth wrote: >> When the userspace program runs the KVM_S390_INTERRUPT ioctl to inject >> an interrupt, we convert them from the legacy struct kvm_s390_interrupt >> to the new struct kvm_s390_irq via the s390int_to_s390irq() function. >> However, this function does not take care of all types of interrupts >> that we can inject into the guest later (see do_inject_vcpu()). Since we >> do not clear out the s390irq values before calling s390int_to_s390irq(), >> there is a chance that we copy unwanted data from the kernel stack >> into the guest memory later if the interrupt data has not been properly >> initialized by s390int_to_s390irq(). >> >> Specifically, the problem exists with the KVM_S390_INT_PFAULT_INIT >> interrupt: s390int_to_s390irq() does not handle it, but the function >> __deliver_pfault_init() will later copy the uninitialized stack data >> from the ext.ext_params2 into the guest memory. >> >> Fix it by handling that interrupt type in s390int_to_s390irq(), too. >> And while we're at it, make sure that s390int_to_s390irq() now >> directly returns -EINVAL for unknown interrupt types, so that we >> do not run into this problem again in case we add more interrupt >> types to do_inject_vcpu() sometime in the future. >> >> Signed-off-by: Thomas Huth >> --- >> arch/s390/kvm/interrupt.c | 10 ++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c >> index 3e7efdd9228a..165dea4c7f19 100644 >> --- a/arch/s390/kvm/interrupt.c >> +++ b/arch/s390/kvm/interrupt.c >> @@ -1960,6 +1960,16 @@ int s390int_to_s390irq(struct kvm_s390_interrupt >> *s390int, >> case KVM_S390_MCHK: >> irq->u.mchk.mcic = s390int->parm64; >> break; >> +case KVM_S390_INT_PFAULT_INIT: >> +irq->u.ext.ext_params = s390int->parm; >> +irq->u.ext.ext_params2 = s390int->parm64; >> +break; >> +case KVM_S390_RESTART: >> +case KVM_S390_INT_CLOCK_COMP: >> +case KVM_S390_INT_CPU_TIMER: >> +break; >> +default: >> +return -EINVAL; >> } >> return 0; >> } >> > > Wouldn't a safe fix be to initialize the struct to zero in the caller? That's of course possible, too. But that means that we always have to zero out the whole structure, so that's a little bit more of overhead (well, it likely doesn't matter for such a legacy ioctl). But the more important question: Do we then still care of fixing the PFAULT_INIT interrupt here? Since it requires a parameter, the "case KVM_S390_INT_PFAULT_INIT:" part would be required here anyway. Thomas
[PATCH] KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT ioctl
When the userspace program runs the KVM_S390_INTERRUPT ioctl to inject an interrupt, we convert them from the legacy struct kvm_s390_interrupt to the new struct kvm_s390_irq via the s390int_to_s390irq() function. However, this function does not take care of all types of interrupts that we can inject into the guest later (see do_inject_vcpu()). Since we do not clear out the s390irq values before calling s390int_to_s390irq(), there is a chance that we copy unwanted data from the kernel stack into the guest memory later if the interrupt data has not been properly initialized by s390int_to_s390irq(). Specifically, the problem exists with the KVM_S390_INT_PFAULT_INIT interrupt: s390int_to_s390irq() does not handle it, but the function __deliver_pfault_init() will later copy the uninitialized stack data from the ext.ext_params2 into the guest memory. Fix it by handling that interrupt type in s390int_to_s390irq(), too. And while we're at it, make sure that s390int_to_s390irq() now directly returns -EINVAL for unknown interrupt types, so that we do not run into this problem again in case we add more interrupt types to do_inject_vcpu() sometime in the future. Signed-off-by: Thomas Huth --- arch/s390/kvm/interrupt.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c index 3e7efdd9228a..165dea4c7f19 100644 --- a/arch/s390/kvm/interrupt.c +++ b/arch/s390/kvm/interrupt.c @@ -1960,6 +1960,16 @@ int s390int_to_s390irq(struct kvm_s390_interrupt *s390int, case KVM_S390_MCHK: irq->u.mchk.mcic = s390int->parm64; break; + case KVM_S390_INT_PFAULT_INIT: + irq->u.ext.ext_params = s390int->parm; + irq->u.ext.ext_params2 = s390int->parm64; + break; + case KVM_S390_RESTART: + case KVM_S390_INT_CLOCK_COMP: + case KVM_S390_INT_CPU_TIMER: + break; + default: + return -EINVAL; } return 0; } -- 2.18.1
[PATCH] KVM: s390: Remove unused parameter from __inject_sigp_restart()
It's not required, so drop it to make it clear that this interrupt
does not have any extra parameters.
Signed-off-by: Thomas Huth
---
arch/s390/kvm/interrupt.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index b5fd6e85657c..3e7efdd9228a 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1477,8 +1477,7 @@ static int __inject_sigp_stop(struct kvm_vcpu *vcpu,
struct kvm_s390_irq *irq)
return 0;
}
-static int __inject_sigp_restart(struct kvm_vcpu *vcpu,
-struct kvm_s390_irq *irq)
+static int __inject_sigp_restart(struct kvm_vcpu *vcpu)
{
struct kvm_s390_local_interrupt *li = >arch.local_int;
@@ -1997,7 +1996,7 @@ static int do_inject_vcpu(struct kvm_vcpu *vcpu, struct
kvm_s390_irq *irq)
rc = __inject_sigp_stop(vcpu, irq);
break;
case KVM_S390_RESTART:
- rc = __inject_sigp_restart(vcpu, irq);
+ rc = __inject_sigp_restart(vcpu);
break;
case KVM_S390_INT_CLOCK_COMP:
rc = __inject_ckc(vcpu);
--
2.18.1
[PATCH] KVM: PPC: Remove superfluous check for non-zero return value
After the kfree()s haven been removed in the previous
commit 9798f4ea71ea ("fix rollback when kvmppc_xive_create fails"),
the code can be simplified even more to simply always "return ret"
now.
Signed-off-by: Thomas Huth
---
arch/powerpc/kvm/book3s_xive.c| 5 +
arch/powerpc/kvm/book3s_xive_native.c | 5 +
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index e3ba67095895..2f6f463fcdfb 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -1986,10 +1986,7 @@ static int kvmppc_xive_create(struct kvm_device *dev,
u32 type)
xive->single_escalation = xive_native_has_single_escalation();
- if (ret)
- return ret;
-
- return 0;
+ return ret;
}
int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_xive_native.c
b/arch/powerpc/kvm/book3s_xive_native.c
index a998823f68a3..7a50772f26fe 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -1089,10 +1089,7 @@ static int kvmppc_xive_native_create(struct kvm_device
*dev, u32 type)
xive->single_escalation = xive_native_has_single_escalation();
xive->ops = _xive_native_ops;
- if (ret)
- return ret;
-
- return 0;
+ return ret;
}
/*
--
2.18.1
Re: [PATCH v2 1/2] KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
On 04/09/2019 11.15, David Hildenbrand wrote:
> On 04.09.19 11:11, Christian Borntraeger wrote:
>>
>>
>> On 04.09.19 11:05, David Hildenbrand wrote:
>>> On 04.09.19 10:51, Thomas Huth wrote:
>>>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>>>> clearly indicates that something went wrong in the KVM userspace
>>>> application. The x86 variant of KVM already contains a check for
>>>> bad bits, so let's do the same on s390x now, too.
>>>>
>>>> Reviewed-by: Janosch Frank
>>>> Reviewed-by: Christian Borntraeger
>>>> Reviewed-by: Cornelia Huck
>>>> Signed-off-by: Thomas Huth
>>>> ---
>>>> arch/s390/include/uapi/asm/kvm.h | 6 ++
>>>> arch/s390/kvm/kvm-s390.c | 4
>>>> 2 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/s390/include/uapi/asm/kvm.h
>>>> b/arch/s390/include/uapi/asm/kvm.h
>>>> index 47104e5b47fd..436ec7636927 100644
>>>> --- a/arch/s390/include/uapi/asm/kvm.h
>>>> +++ b/arch/s390/include/uapi/asm/kvm.h
>>>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>>>> #define KVM_SYNC_GSCB (1UL << 9)
>>>> #define KVM_SYNC_BPBC (1UL << 10)
>>>> #define KVM_SYNC_ETOKEN (1UL << 11)
>>>> +
>>>> +#define KVM_SYNC_S390_VALID_FIELDS \
>>>> + (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>>>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>>>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>>>> +
>>>
>>> We didn't care about the S390 for the actual flags, why care now?
>>
>> I think it makes sense to have the interface as defined as possible. If for
>> some
>> reason userspace sets a wrong bit this would be undetected. If we at a later
>> point
>> in time use that bit this would resultin strange problems.
>
> Not arguing about the concept of checking for valid bits. Was just
> wondering if the "S390" part in the name makes sense at all. But you
> guys seem to have a consent here.
Oh, I guess we both got your question wrong... Well, I don't care too
much whether we've got an "S390" in there or not ... so unless there is
a real good reason to remove it, let's simply keep it this way.
Thomas
Re: [PATCH v2 1/2] KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
On 04/09/2019 11.05, David Hildenbrand wrote:
> On 04.09.19 10:51, Thomas Huth wrote:
>> If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
>> clearly indicates that something went wrong in the KVM userspace
>> application. The x86 variant of KVM already contains a check for
>> bad bits, so let's do the same on s390x now, too.
>>
>> Reviewed-by: Janosch Frank
>> Reviewed-by: Christian Borntraeger
>> Reviewed-by: Cornelia Huck
>> Signed-off-by: Thomas Huth
>> ---
>> arch/s390/include/uapi/asm/kvm.h | 6 ++
>> arch/s390/kvm/kvm-s390.c | 4
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/arch/s390/include/uapi/asm/kvm.h
>> b/arch/s390/include/uapi/asm/kvm.h
>> index 47104e5b47fd..436ec7636927 100644
>> --- a/arch/s390/include/uapi/asm/kvm.h
>> +++ b/arch/s390/include/uapi/asm/kvm.h
>> @@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
>> #define KVM_SYNC_GSCB (1UL << 9)
>> #define KVM_SYNC_BPBC (1UL << 10)
>> #define KVM_SYNC_ETOKEN (1UL << 11)
>> +
>> +#define KVM_SYNC_S390_VALID_FIELDS \
>> +(KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
>> + KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
>> + KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
>> +
>
> We didn't care about the S390 for the actual flags, why care now?
x86 does the same, and we don't want to be worse than x86, do we? ;-)
Honestly, this was just one of the differences that I noticed while
porting the sync_regs_test from x86 to s390x.
Thomas
[PATCH v2 0/2] KVM: s390: Check for invalid bits in kvm_valid_regs and kvm_dirty_regs
Avoid invalid bits in kvm_valid_regs and kvm_dirty_regs on s390x. v2: - Split the single patch from v1 into two separate patches (I've kept the Reviewed-bys from v1, but if you don't agree with the patch description of the 2nd patch, please complain) Thomas Huth (2): KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs KVM: selftests: Test invalid bits in kvm_valid_regs and kvm_dirty_regs on s390x arch/s390/include/uapi/asm/kvm.h | 6 arch/s390/kvm/kvm-s390.c | 4 +++ .../selftests/kvm/s390x/sync_regs_test.c | 30 +++ 3 files changed, 40 insertions(+) -- 2.18.1
[PATCH v2 1/2] KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
clearly indicates that something went wrong in the KVM userspace
application. The x86 variant of KVM already contains a check for
bad bits, so let's do the same on s390x now, too.
Reviewed-by: Janosch Frank
Reviewed-by: Christian Borntraeger
Reviewed-by: Cornelia Huck
Signed-off-by: Thomas Huth
---
arch/s390/include/uapi/asm/kvm.h | 6 ++
arch/s390/kvm/kvm-s390.c | 4
2 files changed, 10 insertions(+)
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 47104e5b47fd..436ec7636927 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
#define KVM_SYNC_GSCB (1UL << 9)
#define KVM_SYNC_BPBC (1UL << 10)
#define KVM_SYNC_ETOKEN (1UL << 11)
+
+#define KVM_SYNC_S390_VALID_FIELDS \
+ (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
+KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
+KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+
/* length and alignment of the sdnx as a power of two */
#define SDNXC 8
#define SDNXL (1UL << SDNXC)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 49d779ae..a7d7dedfe527 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3998,6 +3998,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
struct kvm_run *kvm_run)
if (kvm_run->immediate_exit)
return -EINTR;
+ if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
+ kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
+ return -EINVAL;
+
vcpu_load(vcpu);
if (guestdbg_exit_pending(vcpu)) {
--
2.18.1
[PATCH v2 2/2] KVM: selftests: Test invalid bits in kvm_valid_regs and kvm_dirty_regs on s390x
Now that we disallow invalid bits in kvm_valid_regs and kvm_dirty_regs on s390x, too, we should also check this condition in the selftests. The code has been taken from the x86-version of the sync_regs_test. Reviewed-by: Janosch Frank Reviewed-by: Christian Borntraeger Reviewed-by: Cornelia Huck Signed-off-by: Thomas Huth --- .../selftests/kvm/s390x/sync_regs_test.c | 30 +++ 1 file changed, 30 insertions(+) diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c index bbc93094519b..d5290b4ad636 100644 --- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c +++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c @@ -85,6 +85,36 @@ int main(int argc, char *argv[]) run = vcpu_state(vm, VCPU_ID); + /* Request reading invalid register set from VCPU. */ + run->kvm_valid_regs = INVALID_SYNC_FIELD; + rv = _vcpu_run(vm, VCPU_ID); + TEST_ASSERT(rv < 0 && errno == EINVAL, + "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n", + rv); + vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0; + + run->kvm_valid_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS; + rv = _vcpu_run(vm, VCPU_ID); + TEST_ASSERT(rv < 0 && errno == EINVAL, + "Invalid kvm_valid_regs did not cause expected KVM_RUN error: %d\n", + rv); + vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0; + + /* Request setting invalid register set into VCPU. */ + run->kvm_dirty_regs = INVALID_SYNC_FIELD; + rv = _vcpu_run(vm, VCPU_ID); + TEST_ASSERT(rv < 0 && errno == EINVAL, + "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n", + rv); + vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0; + + run->kvm_dirty_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS; + rv = _vcpu_run(vm, VCPU_ID); + TEST_ASSERT(rv < 0 && errno == EINVAL, + "Invalid kvm_dirty_regs did not cause expected KVM_RUN error: %d\n", + rv); + vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0; + /* Request and verify all valid register sets. */ run->kvm_valid_regs = TEST_SYNC_FIELDS; rv = _vcpu_run(vm, VCPU_ID); -- 2.18.1
[PATCH] KVM: s390: Disallow invalid bits in kvm_valid_regs and kvm_dirty_regs
If unknown bits are set in kvm_valid_regs or kvm_dirty_regs, this
clearly indicates that something went wrong in the KVM userspace
application. The x86 variant of KVM already contains a check for
bad bits (and the corresponding kselftest checks this), so let's
do the same on s390x now, too.
Signed-off-by: Thomas Huth
---
arch/s390/include/uapi/asm/kvm.h | 6
arch/s390/kvm/kvm-s390.c | 4 +++
.../selftests/kvm/s390x/sync_regs_test.c | 30 +++
3 files changed, 40 insertions(+)
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 47104e5b47fd..436ec7636927 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -231,6 +231,12 @@ struct kvm_guest_debug_arch {
#define KVM_SYNC_GSCB (1UL << 9)
#define KVM_SYNC_BPBC (1UL << 10)
#define KVM_SYNC_ETOKEN (1UL << 11)
+
+#define KVM_SYNC_S390_VALID_FIELDS \
+ (KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
+KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
+KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+
/* length and alignment of the sdnx as a power of two */
#define SDNXC 8
#define SDNXL (1UL << SDNXC)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 49d779ae..a7d7dedfe527 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3998,6 +3998,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
struct kvm_run *kvm_run)
if (kvm_run->immediate_exit)
return -EINTR;
+ if (kvm_run->kvm_valid_regs & ~KVM_SYNC_S390_VALID_FIELDS ||
+ kvm_run->kvm_dirty_regs & ~KVM_SYNC_S390_VALID_FIELDS)
+ return -EINVAL;
+
vcpu_load(vcpu);
if (guestdbg_exit_pending(vcpu)) {
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index bbc93094519b..d5290b4ad636 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -85,6 +85,36 @@ int main(int argc, char *argv[])
run = vcpu_state(vm, VCPU_ID);
+ /* Request reading invalid register set from VCPU. */
+ run->kvm_valid_regs = INVALID_SYNC_FIELD;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_valid_regs did not cause expected KVM_RUN
error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
+
+ run->kvm_valid_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_valid_regs did not cause expected KVM_RUN
error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_valid_regs = 0;
+
+ /* Request setting invalid register set into VCPU. */
+ run->kvm_dirty_regs = INVALID_SYNC_FIELD;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_dirty_regs did not cause expected KVM_RUN
error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
+
+ run->kvm_dirty_regs = INVALID_SYNC_FIELD | TEST_SYNC_FIELDS;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv < 0 && errno == EINVAL,
+ "Invalid kvm_dirty_regs did not cause expected KVM_RUN
error: %d\n",
+ rv);
+ vcpu_state(vm, VCPU_ID)->kvm_dirty_regs = 0;
+
/* Request and verify all valid register sets. */
run->kvm_valid_regs = TEST_SYNC_FIELDS;
rv = _vcpu_run(vm, VCPU_ID);
--
2.18.1
Re: [PATCH v3] KVM: selftests: Add a test for the KVM_S390_MEM_OP ioctl
On 29/08/2019 15.26, Christian Borntraeger wrote: > > > On 29.08.19 15:07, Thomas Huth wrote: >> Check that we can write and read the guest memory with this s390x >> ioctl, and that some error cases are handled correctly. >> >> Signed-off-by: Thomas Huth >> --- >> v3: >> - Replaced wrong copy-n-pasted report string with a proper one >> - Check for errno after calling the ioctl with size = 0 > > the test succeeds (as the vmalloc fails) but dmesg then has the warning. > Do we have a chance to parse dmesg somehow? I'm not aware of an easy way to do this from within the KVM selftests. > I will apply this nevertheless for the time being together with the fix. Thanks! Thomas
[PATCH v3] KVM: selftests: Add a test for the KVM_S390_MEM_OP ioctl
Check that we can write and read the guest memory with this s390x
ioctl, and that some error cases are handled correctly.
Signed-off-by: Thomas Huth
---
v3:
- Replaced wrong copy-n-pasted report string with a proper one
- Check for errno after calling the ioctl with size = 0
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/s390x/memop.c | 166 ++
2 files changed, 167 insertions(+)
create mode 100644 tools/testing/selftests/kvm/s390x/memop.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 1b48a94b4350..62c591f87dab 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -32,6 +32,7 @@ TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
+TEST_GEN_PROGS_s390x = s390x/memop
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
diff --git a/tools/testing/selftests/kvm/s390x/memop.c
b/tools/testing/selftests/kvm/s390x/memop.c
new file mode 100644
index ..9edaa9a134ce
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Test for s390x KVM_S390_MEM_OP
+ *
+ * Copyright (C) 2019, Red Hat, Inc.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "test_util.h"
+#include "kvm_util.h"
+
+#define VCPU_ID 1
+
+static uint8_t mem1[65536];
+static uint8_t mem2[65536];
+
+static void guest_code(void)
+{
+ int i;
+
+ for (;;) {
+ for (i = 0; i < sizeof(mem2); i++)
+ mem2[i] = mem1[i];
+ GUEST_SYNC(0);
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ struct kvm_s390_mem_op ksmo;
+ int rv, i, maxsize;
+
+ setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
+
+ maxsize = kvm_check_cap(KVM_CAP_S390_MEM_OP);
+ if (!maxsize) {
+ fprintf(stderr, "CAP_S390_MEM_OP not supported -> skip test\n");
+ exit(KSFT_SKIP);
+ }
+ if (maxsize > sizeof(mem1))
+ maxsize = sizeof(mem1);
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+ run = vcpu_state(vm, VCPU_ID);
+
+ for (i = 0; i < sizeof(mem1); i++)
+ mem1[i] = i * i + i;
+
+ /* Set the first array */
+ ksmo.gaddr = addr_gva2gpa(vm, (uintptr_t)mem1);
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+
+ /* Let the guest code copy the first array to the second */
+ vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC,
+ "Unexpected exit reason: %u (%s)\n",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ memset(mem2, 0xaa, sizeof(mem2));
+
+ /* Get the second array */
+ ksmo.gaddr = (uintptr_t)mem2;
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_READ;
+ ksmo.buf = (uintptr_t)mem2;
+ ksmo.ar = 0;
+ vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+
+ /* Check error conditions - first bad size: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = -1;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == E2BIG, "ioctl allows insane sizes");
+
+ /* Zero size: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = 0;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && (errno == EINVAL || errno == ENOMEM),
+ "ioctl allows 0 as size");
+
+ /* Bad flags: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = -1;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags");
+
+ /* Bad operation: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = -1;
+ ksmo.buf = (uintptr
Re: [PATCH] KVM: s390: improve documentation for S390_MEM_OP
On 29/08/2019 14.47, Cornelia Huck wrote: > Explicitly specify the valid ranges for size and ar, and reword > buf requirements a bit. > > Signed-off-by: Cornelia Huck > --- > supposed to go on top of "KVM: s390: Test for bad access register and > size at the start of S390_MEM_OP" (<20190829122517.31042-1-th...@redhat.com>) > --- > Documentation/virt/kvm/api.txt | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/Documentation/virt/kvm/api.txt b/Documentation/virt/kvm/api.txt > index 2d067767b617..76c9d6fdbfdb 100644 > --- a/Documentation/virt/kvm/api.txt > +++ b/Documentation/virt/kvm/api.txt > @@ -3079,12 +3079,14 @@ This exception is also raised directly at the > corresponding VCPU if the > flag KVM_S390_MEMOP_F_INJECT_EXCEPTION is set in the "flags" field. > > The start address of the memory region has to be specified in the "gaddr" > -field, and the length of the region in the "size" field. "buf" is the buffer > -supplied by the userspace application where the read data should be written > -to for KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written > -is stored for a KVM_S390_MEMOP_LOGICAL_WRITE. "buf" is unused and can be NULL > -when KVM_S390_MEMOP_F_CHECK_ONLY is specified. "ar" designates the access > -register number to be used. > +field, and the length of the region in the "size" field (which must not > +be 0). The maximum value for "size" can be obtained by checking the > +KVM_CAP_S390_MEM_OP capability. "buf" is the buffer supplied by the > +userspace application where the read data should be written to for > +KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written is > +stored for a KVM_S390_MEMOP_LOGICAL_WRITE. When KVM_S390_MEMOP_F_CHECK_ONLY > +is specified, "buf" is unused and can be NULL. "ar" designates the access > +register number to be used; the valid range is 0..15. > > The "reserved" field is meant for future extensions. It is not used by > KVM with the currently defined set of flags. > Reviewed-by: Thomas Huth
Re: [PATCH v2] KVM: selftests: Add a test for the KVM_S390_MEM_OP ioctl
On 29/08/2019 14.21, Janosch Frank wrote: > On 8/29/19 2:14 PM, Thomas Huth wrote: >> Check that we can write and read the guest memory with this s390x >> ioctl, and that some error cases are handled correctly. >> >> Signed-off-by: Thomas Huth >> --- >> v2: Check the ioctl also with "size" set to 0 >> > [...] >> + >> +/* Zero size: */ >> +ksmo.gaddr = (uintptr_t)mem1; >> +ksmo.flags = 0; >> +ksmo.size = 0; >> +ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE; >> +ksmo.buf = (uintptr_t)mem1; >> +ksmo.ar = 0; >> +rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, ); >> +TEST_ASSERT(rv == -1, "ioctl allows 0 as size"); > > Test for errno == -EINVAL? Assuming that my "Test for bad access register and size at the start of S390_MEM_OP" goes in first, yes, I can add that check. Otherwise, the current kernel still returns ENOMEM. >> + >> +/* Bad flags: */ >> +ksmo.gaddr = (uintptr_t)mem1; >> +ksmo.flags = -1; >> +ksmo.size = maxsize; >> +ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE; >> +ksmo.buf = (uintptr_t)mem1; >> +ksmo.ar = 0; >> +rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, ); >> +TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags?"); >> + >> +/* Bad operation: */ >> +ksmo.gaddr = (uintptr_t)mem1; >> +ksmo.flags = 0; >> +ksmo.size = maxsize; >> +ksmo.op = -1; >> +ksmo.buf = (uintptr_t)mem1; >> +ksmo.ar = 0; >> +rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, ); >> +TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags?"); > > Wrong report string Oops, stupid copy-n-paste bug ... I'll fix it in v3... Thanks, Thomas signature.asc Description: OpenPGP digital signature
[PATCH v2] KVM: s390: Test for bad access register and size at the start of S390_MEM_OP
If the KVM_S390_MEM_OP ioctl is called with an access register >= 16, then there is certainly a bug in the calling userspace application. We check for wrong access registers, but only if the vCPU was already in the access register mode before (i.e. the SIE block has recorded it). The check is also buried somewhere deep in the calling chain (in the function ar_translation()), so this is somewhat hard to find. It's better to always report an error to the userspace in case this field is set wrong, and it's safer in the KVM code if we block wrong values here early instead of relying on a check somewhere deep down the calling chain, so let's add another check to kvm_s390_guest_mem_op() directly. We also should check that the "size" is non-zero here (thanks to Janosch Frank for the hint!). If we do not check the size, we could call vmalloc() with this 0 value, and this will cause a kernel warning. Signed-off-by: Thomas Huth --- v2: Check mop->size to be non-zero arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index f329dcb3f44c..49d779ae 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4255,7 +4255,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu, const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION | KVM_S390_MEMOP_F_CHECK_ONLY; - if (mop->flags & ~supported_flags) + if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS || !mop->size) return -EINVAL; if (mop->size > MEM_OP_MAX_SIZE) -- 2.18.1
[PATCH v2] KVM: selftests: Add a test for the KVM_S390_MEM_OP ioctl
Check that we can write and read the guest memory with this s390x
ioctl, and that some error cases are handled correctly.
Signed-off-by: Thomas Huth
---
v2: Check the ioctl also with "size" set to 0
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/s390x/memop.c | 165 ++
2 files changed, 166 insertions(+)
create mode 100644 tools/testing/selftests/kvm/s390x/memop.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 1b48a94b4350..62c591f87dab 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -32,6 +32,7 @@ TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
+TEST_GEN_PROGS_s390x = s390x/memop
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
diff --git a/tools/testing/selftests/kvm/s390x/memop.c
b/tools/testing/selftests/kvm/s390x/memop.c
new file mode 100644
index ..e6a65f9e48ca
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Test for s390x KVM_S390_MEM_OP
+ *
+ * Copyright (C) 2019, Red Hat, Inc.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "test_util.h"
+#include "kvm_util.h"
+
+#define VCPU_ID 1
+
+static uint8_t mem1[65536];
+static uint8_t mem2[65536];
+
+static void guest_code(void)
+{
+ int i;
+
+ for (;;) {
+ for (i = 0; i < sizeof(mem2); i++)
+ mem2[i] = mem1[i];
+ GUEST_SYNC(0);
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ struct kvm_s390_mem_op ksmo;
+ int rv, i, maxsize;
+
+ setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
+
+ maxsize = kvm_check_cap(KVM_CAP_S390_MEM_OP);
+ if (!maxsize) {
+ fprintf(stderr, "CAP_S390_MEM_OP not supported -> skip test\n");
+ exit(KSFT_SKIP);
+ }
+ if (maxsize > sizeof(mem1))
+ maxsize = sizeof(mem1);
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+ run = vcpu_state(vm, VCPU_ID);
+
+ for (i = 0; i < sizeof(mem1); i++)
+ mem1[i] = i * i + i;
+
+ /* Set the first array */
+ ksmo.gaddr = addr_gva2gpa(vm, (uintptr_t)mem1);
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+
+ /* Let the guest code copy the first array to the second */
+ vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC,
+ "Unexpected exit reason: %u (%s)\n",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ memset(mem2, 0xaa, sizeof(mem2));
+
+ /* Get the second array */
+ ksmo.gaddr = (uintptr_t)mem2;
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_READ;
+ ksmo.buf = (uintptr_t)mem2;
+ ksmo.ar = 0;
+ vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+
+ /* Check error conditions - first bad size: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = -1;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == E2BIG, "ioctl allows insane sizes");
+
+ /* Zero size: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = 0;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1, "ioctl allows 0 as size");
+
+ /* Bad flags: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = -1;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags?");
+
+ /* Bad operation: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = -1;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 &
Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
On 29/08/2019 13.15, Janosch Frank wrote: [...] > By the way, I think we want to check mop->size for 0 before giving it to > vmalloc and working with it. You're right! This currently triggers a kernel warning message with a Call Trace! I'll add a check to my new memop selftest and send a patch... Thomas signature.asc Description: OpenPGP digital signature
Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
On 29/08/2019 13.18, Cornelia Huck wrote: [...] > > Btw: should Documentation/virt/kvm/api.txt spell out the valid range > for ar explicitly? > That certainly would not hurt. Care to send a patch, or shall I assemble one? Thomas
[PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
If the KVM_S390_MEM_OP ioctl is called with an access register >= 16, then there is certainly a bug in the calling userspace application. We check for wrong access registers, but only if the vCPU was already in the access register mode before (i.e. the SIE block has recorded it). The check is also buried somewhere deep in the calling chain (in the function ar_translation()), so this is somewhat hard to find. It's better to always report an error to the userspace in case this field is set wrong, and it's safer in the KVM code if we block wrong values here early instead of relying on a check somewhere deep down the calling chain, so let's add another check to kvm_s390_guest_mem_op() directly. Signed-off-by: Thomas Huth --- arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index f329dcb3f44c..725690853cbd 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4255,7 +4255,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu, const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION | KVM_S390_MEMOP_F_CHECK_ONLY; - if (mop->flags & ~supported_flags) + if (mop->flags & ~supported_flags || mop->ar >= NUM_ACRS) return -EINVAL; if (mop->size > MEM_OP_MAX_SIZE) -- 2.18.1
[PATCH] KVM: selftests: Add a test for the KVM_S390_MEM_OP ioctl
Check that we can write and read the guest memory with this s390x
ioctl, and that some error cases are handled correctly.
Signed-off-by: Thomas Huth
---
This test uses the ucall() interface, so this patch needs to be applied
on top of my "Implement ucall() for s390x" patch (which is not merged to
master yet)
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/s390x/memop.c | 155 ++
2 files changed, 156 insertions(+)
create mode 100644 tools/testing/selftests/kvm/s390x/memop.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 1b48a94b4350..62c591f87dab 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -32,6 +32,7 @@ TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
+TEST_GEN_PROGS_s390x = s390x/memop
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
diff --git a/tools/testing/selftests/kvm/s390x/memop.c
b/tools/testing/selftests/kvm/s390x/memop.c
new file mode 100644
index ..25b100d9fdda
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Test for s390x KVM_S390_MEM_OP
+ *
+ * Copyright (C) 2019, Red Hat, Inc.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "test_util.h"
+#include "kvm_util.h"
+
+#define VCPU_ID 1
+
+static uint8_t mem1[65536];
+static uint8_t mem2[65536];
+
+static void guest_code(void)
+{
+ int i;
+
+ for (;;) {
+ for (i = 0; i < sizeof(mem2); i++)
+ mem2[i] = mem1[i];
+ GUEST_SYNC(0);
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ struct kvm_s390_mem_op ksmo;
+ int rv, i, maxsize;
+
+ setbuf(stdout, NULL); /* Tell stdout not to buffer its content */
+
+ maxsize = kvm_check_cap(KVM_CAP_S390_MEM_OP);
+ if (!maxsize) {
+ fprintf(stderr, "CAP_S390_MEM_OP not supported -> skip test\n");
+ exit(KSFT_SKIP);
+ }
+ if (maxsize > sizeof(mem1))
+ maxsize = sizeof(mem1);
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+ run = vcpu_state(vm, VCPU_ID);
+
+ for (i = 0; i < sizeof(mem1); i++)
+ mem1[i] = i * i + i;
+
+ /* Set the first array */
+ ksmo.gaddr = addr_gva2gpa(vm, (uintptr_t)mem1);
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+
+ /* Let the guest code copy the first array to the second */
+ vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC,
+ "Unexpected exit reason: %u (%s)\n",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+
+ memset(mem2, 0xaa, sizeof(mem2));
+
+ /* Get the second array */
+ ksmo.gaddr = (uintptr_t)mem2;
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_READ;
+ ksmo.buf = (uintptr_t)mem2;
+ ksmo.ar = 0;
+ vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+
+ TEST_ASSERT(!memcmp(mem1, mem2, maxsize),
+ "Memory contents do not match!");
+
+ /* Check error conditions - first bad size: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = -1;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == E2BIG, "ioctl allows insane sizes");
+
+ /* Bad flags: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = -1;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S390_MEMOP_LOGICAL_WRITE;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags?");
+
+ /* Bad operation: */
+ ksmo.gaddr = (uintptr_t)mem1;
+ ksmo.flags = 0;
+ ksmo.size = maxsize;
+ ksmo.op = -1;
+ ksmo.buf = (uintptr_t)mem1;
+ ksmo.ar = 0;
+ rv = _vcpu_ioctl(vm, VCPU_ID, KVM_S390_MEM_OP, );
+ TEST_ASSERT(rv == -1 && errno == EINVAL, "ioctl allows all flags?");
+
+ /* Bad guest address: */
+ ksmo.gaddr = ~0xfffUL;
+ ksmo.flags = KVM_S390_MEMOP_F_CHECK_ONLY;
+ ksmo.size = maxsize;
+ ksmo.op = KVM_S
[PATCH] powerpc: Replace GPL boilerplate with SPDX identifiers
The FSF does not reside in "675 Mass Ave, Cambridge" anymore... let's simply use proper SPDX identifiers instead. Signed-off-by: Thomas Huth --- arch/powerpc/include/uapi/asm/spu_info.h | 14 -- arch/powerpc/kernel/eeh_driver.c | 18 +- arch/powerpc/kernel/eeh_sysfs.c| 18 +- arch/powerpc/platforms/pseries/pci_dlpar.c | 18 +- 4 files changed, 3 insertions(+), 65 deletions(-) diff --git a/arch/powerpc/include/uapi/asm/spu_info.h b/arch/powerpc/include/uapi/asm/spu_info.h index cabfcbba9eac..45f97150587b 100644 --- a/arch/powerpc/include/uapi/asm/spu_info.h +++ b/arch/powerpc/include/uapi/asm/spu_info.h @@ -5,20 +5,6 @@ * (C) Copyright 2006 IBM Corp. * * Author: Dwayne Grant McConnell - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ #ifndef _UAPI_SPU_INFO_H diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c index 89623962c727..3bb27ded9daa 100644 --- a/arch/powerpc/kernel/eeh_driver.c +++ b/arch/powerpc/kernel/eeh_driver.c @@ -1,25 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* * PCI Error Recovery Driver for RPA-compliant PPC64 platform. * Copyright IBM Corp. 2004 2005 * Copyright Linas Vepstas 2004, 2005 * - * All rights reserved. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or (at - * your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or - * NON INFRINGEMENT. See the GNU General Public License for more - * details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - * * Send comments and feedback to Linas Vepstas */ #include diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c index 3fa04dda1737..ab44d965a53c 100644 --- a/arch/powerpc/kernel/eeh_sysfs.c +++ b/arch/powerpc/kernel/eeh_sysfs.c @@ -1,25 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* * Sysfs entries for PCI Error Recovery for PAPR-compliant platform. * Copyright IBM Corporation 2007 * Copyright Linas Vepstas 2007 * - * All rights reserved. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or (at - * your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or - * NON INFRINGEMENT. See the GNU General Public License for more - * details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. - * * Send comments and feedback to Linas Vepstas */ #include diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c index 561917fa54a8..361986e4354e 100644 --- a/arch/powerpc/platforms/pseries/pci_dlpar.c +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* * PCI Dynamic LPAR, PCI Hot Plug and PCI EEH recovery code * for RPA-compliant PPC64 platform. @@ -6,23 +7,6 @@ * * Updates, 2005, John Rose * Updates, 2005, Linas Vepstas - * - * All rights reserved. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or (at - * your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY OR FITNESS FOR A
Re: [PATCH] KVM: selftests: Detect max PA width from cpuid
On 26/08/2019 09.57, Peter Xu wrote: > The dirty_log_test is failing on some old machines like Xeon E3-1220 > with tripple faults when writting to the tracked memory region: > > Test iterations: 32, interval: 10 (ms) > Testing guest mode: PA-bits:52, VA-bits:48, 4K pages > guest physical test memory offset: 0x7fbffef000 > Test Assertion Failure > dirty_log_test.c:138: false > pid=6137 tid=6139 - Success > 1 0x00401ca1: vcpu_worker at dirty_log_test.c:138 > 2 0x7f3dd9e392dd: ?? ??:0 > 3 0x7f3dd9b6a132: ?? ??:0 > Invalid guest sync status: exit_reason=SHUTDOWN > > It's because previously we moved the testing memory region from a > static place (1G) to the top of the system's physical address space, > meanwhile we stick to 39 bits PA for all the x86_64 machines. That's > not true for machines like Xeon E3-1220 where it only supports 36. > > Let's unbreak this test by dynamically detect PA width from CPUID > 0x8008. Meanwhile, even allow kvm_get_supported_cpuid_index() to > fail. I don't know whether that could be useful because I think > 0x8008 should be there for all x86_64 hosts, but I also think it's > not really helpful to assert in the kvm_get_supported_cpuid_index(). [...] > diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c > b/tools/testing/selftests/kvm/lib/x86_64/processor.c > index 6cb34a0fa200..9de2fd310ac8 100644 > --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c > +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c > @@ -760,9 +760,6 @@ kvm_get_supported_cpuid_index(uint32_t function, uint32_t > index) > break; > } > } > - > - TEST_ASSERT(entry, "Guest CPUID entry not found: (EAX=%x, ECX=%x).", > - function, index); > return entry; > } You should also adjust the comment of the function. It currently says "Never returns NULL". Not it can return NULL. And maybe add a TEST_ASSERT() to the other callers instead, which do not expect a NULL to be returned? Thomas
[PATCH v3 2/3] KVM: selftests: Implement ucall() for s390x
On s390x, we can neither exit via PIO nor MMIO, but have to use an
instruction like DIAGNOSE. Now that ucall() is implemented, we can
use it in the sync_reg_test on s390x, too.
Reviewed-by: Andrew Jones
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 2 +-
tools/testing/selftests/kvm/lib/s390x/ucall.c | 56 +++
.../selftests/kvm/s390x/sync_regs_test.c | 6 +-
3 files changed, 61 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/kvm/lib/s390x/ucall.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index a51e3b83df40..75ea1ecbf85a 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -10,7 +10,7 @@ UNAME_M := $(shell uname -m)
LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c
LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/ucall.c
LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
-LIBKVM_s390x = lib/s390x/processor.c
+LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
diff --git a/tools/testing/selftests/kvm/lib/s390x/ucall.c
b/tools/testing/selftests/kvm/lib/s390x/ucall.c
new file mode 100644
index ..fd589dc9bfab
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/s390x/ucall.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ucall support. A ucall is a "hypercall to userspace".
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ */
+#include "kvm_util.h"
+
+void ucall_init(struct kvm_vm *vm, void *arg)
+{
+}
+
+void ucall_uninit(struct kvm_vm *vm)
+{
+}
+
+void ucall(uint64_t cmd, int nargs, ...)
+{
+ struct ucall uc = {
+ .cmd = cmd,
+ };
+ va_list va;
+ int i;
+
+ nargs = nargs <= UCALL_MAX_ARGS ? nargs : UCALL_MAX_ARGS;
+
+ va_start(va, nargs);
+ for (i = 0; i < nargs; ++i)
+ uc.args[i] = va_arg(va, uint64_t);
+ va_end(va);
+
+ /* Exit via DIAGNOSE 0x501 (normally used for breakpoints) */
+ asm volatile ("diag 0,%0,0x501" : : "a"() : "memory");
+}
+
+uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
+{
+ struct kvm_run *run = vcpu_state(vm, vcpu_id);
+ struct ucall ucall = {};
+
+ if (run->exit_reason == KVM_EXIT_S390_SIEIC &&
+ run->s390_sieic.icptcode == 4 &&
+ (run->s390_sieic.ipa >> 8) == 0x83 &&/* 0x83 means DIAGNOSE */
+ (run->s390_sieic.ipb >> 16) == 0x501) {
+ int reg = run->s390_sieic.ipa & 0xf;
+
+ memcpy(, addr_gva2hva(vm, run->s.regs.gprs[reg]),
+ sizeof(ucall));
+
+ vcpu_run_complete_io(vm, vcpu_id);
+ if (uc)
+ memcpy(uc, , sizeof(ucall));
+ }
+
+ return ucall.cmd;
+}
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index e85ff0d69548..bbc93094519b 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -25,9 +25,11 @@
static void guest_code(void)
{
+ register u64 stage asm("11") = 0;
+
for (;;) {
- asm volatile ("diag 0,0,0x501");
- asm volatile ("ahi 11,1");
+ GUEST_SYNC(0);
+ asm volatile ("ahi %0,1" : : "r"(stage));
}
}
--
2.21.0
[PATCH v3 1/3] KVM: selftests: Split ucall.c into architecture specific files
The way we exit from a guest to userspace is very specific to the
architecture: On x86, we use PIO, on aarch64 we are using MMIO and on
s390x we're going to use an instruction instead. The possibility to
select a type via the ucall_type_t enum is currently also completely
unused, so the code in ucall.c currently looks more complex than
required. Let's split this up into architecture specific ucall.c
files instead, so we can get rid of the #ifdefs and the unnecessary
ucall_type_t handling.
Reviewed-by: Andrew Jones
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 6 +-
tools/testing/selftests/kvm/dirty_log_test.c | 2 +-
.../testing/selftests/kvm/include/kvm_util.h | 8 +-
.../testing/selftests/kvm/lib/aarch64/ucall.c | 112 +
tools/testing/selftests/kvm/lib/ucall.c | 157 --
.../testing/selftests/kvm/lib/x86_64/ucall.c | 56 +++
6 files changed, 173 insertions(+), 168 deletions(-)
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/ucall.c
delete mode 100644 tools/testing/selftests/kvm/lib/ucall.c
create mode 100644 tools/testing/selftests/kvm/lib/x86_64/ucall.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index ba7849751989..a51e3b83df40 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -7,9 +7,9 @@ top_srcdir = ../../../..
KSFT_KHDR_INSTALL := 1
UNAME_M := $(shell uname -m)
-LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/ucall.c
lib/sparsebit.c
-LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c
-LIBKVM_aarch64 = lib/aarch64/processor.c
+LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c
+LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/ucall.c
+LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
LIBKVM_s390x = lib/s390x/processor.c
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c
b/tools/testing/selftests/kvm/dirty_log_test.c
index ceb52b952637..5d5ae1be4984 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -337,7 +337,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long
iterations,
vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
#endif
#ifdef __aarch64__
- ucall_init(vm, UCALL_MMIO, NULL);
+ ucall_init(vm, NULL);
#endif
/* Export the shared variables to the guest */
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index e0e66b115ef2..5463b7896a0a 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -165,12 +165,6 @@ int vm_create_device(struct kvm_vm *vm, struct
kvm_create_device *cd);
memcpy(&(g), _p, sizeof(g));\
})
-/* ucall implementation types */
-typedef enum {
- UCALL_PIO,
- UCALL_MMIO,
-} ucall_type_t;
-
/* Common ucalls */
enum {
UCALL_NONE,
@@ -186,7 +180,7 @@ struct ucall {
uint64_t args[UCALL_MAX_ARGS];
};
-void ucall_init(struct kvm_vm *vm, ucall_type_t type, void *arg);
+void ucall_init(struct kvm_vm *vm, void *arg);
void ucall_uninit(struct kvm_vm *vm);
void ucall(uint64_t cmd, int nargs, ...);
uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc);
diff --git a/tools/testing/selftests/kvm/lib/aarch64/ucall.c
b/tools/testing/selftests/kvm/lib/aarch64/ucall.c
new file mode 100644
index ..6cd91970fbad
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/ucall.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ucall support. A ucall is a "hypercall to userspace".
+ *
+ * Copyright (C) 2018, Red Hat, Inc.
+ */
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+
+static vm_vaddr_t *ucall_exit_mmio_addr;
+
+static bool ucall_mmio_init(struct kvm_vm *vm, vm_paddr_t gpa)
+{
+ if (kvm_userspace_memory_region_find(vm, gpa, gpa + 1))
+ return false;
+
+ virt_pg_map(vm, gpa, gpa, 0);
+
+ ucall_exit_mmio_addr = (vm_vaddr_t *)gpa;
+ sync_global_to_guest(vm, ucall_exit_mmio_addr);
+
+ return true;
+}
+
+void ucall_init(struct kvm_vm *vm, void *arg)
+{
+ vm_paddr_t gpa, start, end, step, offset;
+ unsigned int bits;
+ bool ret;
+
+ if (arg) {
+ gpa = (vm_paddr_t)arg;
+ ret = ucall_mmio_init(vm, gpa);
+ TEST_ASSERT(ret, "Can't set ucall mmio address to %lx", gpa);
+ return;
+ }
+
+ /*
+* Find an address within the allowed physical and virtual address
+* spaces, that does _not_ have a KVM memory region associated with
+* it. Identity mapping an address like this allows the guest to
+* a
[PATCH v3 0/3] KVM: selftests: Enable ucall and dirty_log_test on s390x
Implement the ucall() interface on s390x to be able to use the dirty_log_test KVM selftest on s390x, too. v3: - Fix compilation issue on aarch64 (thanks to Andrew for testing it!) - Added Reviewed-bys v2: - Split up ucall.c into architecture specific files - Removed some #ifdef __s390x__ in the dirty_log patch Thomas Huth (3): KVM: selftests: Split ucall.c into architecture specific files KVM: selftests: Implement ucall() for s390x KVM: selftests: Enable dirty_log_test on s390x tools/testing/selftests/kvm/Makefile | 9 +- tools/testing/selftests/kvm/dirty_log_test.c | 61 ++- .../testing/selftests/kvm/include/kvm_util.h | 8 +- .../testing/selftests/kvm/lib/aarch64/ucall.c | 112 + tools/testing/selftests/kvm/lib/s390x/ucall.c | 56 +++ tools/testing/selftests/kvm/lib/ucall.c | 157 -- .../testing/selftests/kvm/lib/x86_64/ucall.c | 56 +++ .../selftests/kvm/s390x/sync_regs_test.c | 6 +- 8 files changed, 287 insertions(+), 178 deletions(-) create mode 100644 tools/testing/selftests/kvm/lib/aarch64/ucall.c create mode 100644 tools/testing/selftests/kvm/lib/s390x/ucall.c delete mode 100644 tools/testing/selftests/kvm/lib/ucall.c create mode 100644 tools/testing/selftests/kvm/lib/x86_64/ucall.c -- 2.21.0
[PATCH v3 3/3] KVM: selftests: Enable dirty_log_test on s390x
To run the dirty_log_test on s390x, we have to make sure that we
access the dirty log bitmap with little endian byte ordering and
we have to properly align the memslot of the guest.
Also all dirty bits of a segment are set once on s390x when one
of the pages of a segment are written to for the first time, so
we have to make sure that we touch all pages during the first
iteration to keep the test in sync here.
DEFAULT_GUEST_TEST_MEM needs an adjustment, too. On some s390x
distributions, the ELF binary is linked to address 0x8000,
so we have to avoid that our test region overlaps into this area.
0xc000 seems to be a good alternative that should work on x86
and aarch64, too.
Acked-by: Paolo Bonzini
Reviewed-by: Andrew Jones
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/dirty_log_test.c | 59 +---
2 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 75ea1ecbf85a..1b48a94b4350 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -33,6 +33,7 @@ TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c
b/tools/testing/selftests/kvm/dirty_log_test.c
index 5d5ae1be4984..dc3346e090f5 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -26,8 +26,8 @@
/* The memory slot index to track dirty pages */
#define TEST_MEM_SLOT_INDEX1
-/* Default guest test memory offset, 1G */
-#define DEFAULT_GUEST_TEST_MEM 0x4000
+/* Default guest test virtual memory offset */
+#define DEFAULT_GUEST_TEST_MEM 0xc000
/* How many pages to dirty for each guest loop */
#define TEST_PAGES_PER_LOOP1024
@@ -38,6 +38,27 @@
/* Interval for each host loop (ms) */
#define TEST_HOST_LOOP_INTERVAL10UL
+/* Dirty bitmaps are always little endian, so we need to swap on big endian */
+#if defined(__s390x__)
+# define BITOP_LE_SWIZZLE ((BITS_PER_LONG-1) & ~0x7)
+# define test_bit_le(nr, addr) \
+ test_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define set_bit_le(nr, addr) \
+ set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define clear_bit_le(nr, addr) \
+ clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define test_and_set_bit_le(nr, addr) \
+ test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define test_and_clear_bit_le(nr, addr) \
+ test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+#else
+# define test_bit_le test_bit
+# define set_bit_leset_bit
+# define clear_bit_le clear_bit
+# define test_and_set_bit_le test_and_set_bit
+# define test_and_clear_bit_le test_and_clear_bit
+#endif
+
/*
* Guest/Host shared variables. Ensure addr_gva2hva() and/or
* sync_global_to/from_guest() are used when accessing from
@@ -69,11 +90,23 @@ static uint64_t guest_test_virt_mem =
DEFAULT_GUEST_TEST_MEM;
*/
static void guest_code(void)
{
+ uint64_t addr;
int i;
+ /*
+* On s390x, all pages of a 1M segment are initially marked as dirty
+* when a page of the segment is written to for the very first time.
+* To compensate this specialty in this test, we need to touch all
+* pages during the first iteration.
+*/
+ for (i = 0; i < guest_num_pages; i++) {
+ addr = guest_test_virt_mem + i * guest_page_size;
+ *(uint64_t *)addr = READ_ONCE(iteration);
+ }
+
while (true) {
for (i = 0; i < TEST_PAGES_PER_LOOP; i++) {
- uint64_t addr = guest_test_virt_mem;
+ addr = guest_test_virt_mem;
addr += (READ_ONCE(random_array[i]) % guest_num_pages)
* guest_page_size;
addr &= ~(host_page_size - 1);
@@ -158,15 +191,15 @@ static void vm_dirty_log_verify(unsigned long *bmap)
value_ptr = host_test_mem + page * host_page_size;
/* If this is a special page that we were tracking... */
- if (test_and_clear_bit(page, host_bmap_track)) {
+ if (test_and_clear_bit_le(page, host_bmap_track)) {
host_track_next_count++;
- TEST_ASSERT(test_bit(page, bmap),
+ TEST_ASSERT(test_bit_le(page, bmap),
"Page %"PRIu64" should have its dirty bit "
"set in this iteration but it is missing",
page);
[PATCH] KVM: selftests: Update gitignore file for latest changes
The kvm_create_max_vcpus test has been moved to the main directory, and sync_regs_test is now available on s390x, too. Signed-off-by: Thomas Huth --- tools/testing/selftests/kvm/.gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore index 41266af0d3dc..b35da375530a 100644 --- a/tools/testing/selftests/kvm/.gitignore +++ b/tools/testing/selftests/kvm/.gitignore @@ -1,7 +1,7 @@ +/s390x/sync_regs_test /x86_64/cr4_cpuid_sync_test /x86_64/evmcs_test /x86_64/hyperv_cpuid -/x86_64/kvm_create_max_vcpus /x86_64/mmio_warning_test /x86_64/platform_info_test /x86_64/set_sregs_test @@ -13,3 +13,4 @@ /x86_64/vmx_tsc_adjust_test /clear_dirty_log_test /dirty_log_test +/kvm_create_max_vcpus -- 2.21.0
Re: [PATCH v2 0/3] KVM: selftests: Enable ucall and dirty_log_test on s390x
On 31/07/2019 15.32, Thomas Huth wrote: > Implement the ucall() interface on s390x to be able to use the > dirty_log_test KVM selftest on s390x, too. > > v2: > - Split up ucall.c into architecture specific files > - Removed some #ifdef __s390x__ in the dirty_log patch > > Thomas Huth (3): > KVM: selftests: Split ucall.c into architecture specific files I forgot to say: I only checked x86 and s390x ... Andrew, could you please check that the dirty_log_test still builds and runs fine on aarch64 after applying my patches? (Sorry, I tried to get a aarch64 machine here, but so far I failed...) Thanks, Thomas
[PATCH v2 0/3] KVM: selftests: Enable ucall and dirty_log_test on s390x
Implement the ucall() interface on s390x to be able to use the dirty_log_test KVM selftest on s390x, too. v2: - Split up ucall.c into architecture specific files - Removed some #ifdef __s390x__ in the dirty_log patch Thomas Huth (3): KVM: selftests: Split ucall.c into architecture specific files KVM: selftests: Implement ucall() for s390x KVM: selftests: Enable dirty_log_test on s390x tools/testing/selftests/kvm/Makefile | 9 +- tools/testing/selftests/kvm/dirty_log_test.c | 61 ++- .../testing/selftests/kvm/include/kvm_util.h | 8 +- .../testing/selftests/kvm/lib/aarch64/ucall.c | 112 + tools/testing/selftests/kvm/lib/s390x/ucall.c | 56 +++ tools/testing/selftests/kvm/lib/ucall.c | 157 -- .../testing/selftests/kvm/lib/x86_64/ucall.c | 56 +++ .../selftests/kvm/s390x/sync_regs_test.c | 6 +- 8 files changed, 287 insertions(+), 178 deletions(-) create mode 100644 tools/testing/selftests/kvm/lib/aarch64/ucall.c create mode 100644 tools/testing/selftests/kvm/lib/s390x/ucall.c delete mode 100644 tools/testing/selftests/kvm/lib/ucall.c create mode 100644 tools/testing/selftests/kvm/lib/x86_64/ucall.c -- 2.21.0
[PATCH v2 3/3] KVM: selftests: Enable dirty_log_test on s390x
To run the dirty_log_test on s390x, we have to make sure that we
access the dirty log bitmap with little endian byte ordering and
we have to properly align the memslot of the guest.
Also all dirty bits of a segment are set once on s390x when one
of the pages of a segment are written to for the first time, so
we have to make sure that we touch all pages during the first
iteration to keep the test in sync here.
DEFAULT_GUEST_TEST_MEM needs an adjustment, too. On some s390x
distributions, the ELF binary is linked to address 0x8000,
so we have to avoid that our test region overlaps into this area.
0xc000 seems to be a good alternative that should work on x86
and aarch64, too.
Acked-by: Paolo Bonzini
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/dirty_log_test.c | 59 +---
2 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 75ea1ecbf85a..1b48a94b4350 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -33,6 +33,7 @@ TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c
b/tools/testing/selftests/kvm/dirty_log_test.c
index 5d5ae1be4984..dc3346e090f5 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -26,8 +26,8 @@
/* The memory slot index to track dirty pages */
#define TEST_MEM_SLOT_INDEX1
-/* Default guest test memory offset, 1G */
-#define DEFAULT_GUEST_TEST_MEM 0x4000
+/* Default guest test virtual memory offset */
+#define DEFAULT_GUEST_TEST_MEM 0xc000
/* How many pages to dirty for each guest loop */
#define TEST_PAGES_PER_LOOP1024
@@ -38,6 +38,27 @@
/* Interval for each host loop (ms) */
#define TEST_HOST_LOOP_INTERVAL10UL
+/* Dirty bitmaps are always little endian, so we need to swap on big endian */
+#if defined(__s390x__)
+# define BITOP_LE_SWIZZLE ((BITS_PER_LONG-1) & ~0x7)
+# define test_bit_le(nr, addr) \
+ test_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define set_bit_le(nr, addr) \
+ set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define clear_bit_le(nr, addr) \
+ clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define test_and_set_bit_le(nr, addr) \
+ test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define test_and_clear_bit_le(nr, addr) \
+ test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+#else
+# define test_bit_le test_bit
+# define set_bit_leset_bit
+# define clear_bit_le clear_bit
+# define test_and_set_bit_le test_and_set_bit
+# define test_and_clear_bit_le test_and_clear_bit
+#endif
+
/*
* Guest/Host shared variables. Ensure addr_gva2hva() and/or
* sync_global_to/from_guest() are used when accessing from
@@ -69,11 +90,23 @@ static uint64_t guest_test_virt_mem =
DEFAULT_GUEST_TEST_MEM;
*/
static void guest_code(void)
{
+ uint64_t addr;
int i;
+ /*
+* On s390x, all pages of a 1M segment are initially marked as dirty
+* when a page of the segment is written to for the very first time.
+* To compensate this specialty in this test, we need to touch all
+* pages during the first iteration.
+*/
+ for (i = 0; i < guest_num_pages; i++) {
+ addr = guest_test_virt_mem + i * guest_page_size;
+ *(uint64_t *)addr = READ_ONCE(iteration);
+ }
+
while (true) {
for (i = 0; i < TEST_PAGES_PER_LOOP; i++) {
- uint64_t addr = guest_test_virt_mem;
+ addr = guest_test_virt_mem;
addr += (READ_ONCE(random_array[i]) % guest_num_pages)
* guest_page_size;
addr &= ~(host_page_size - 1);
@@ -158,15 +191,15 @@ static void vm_dirty_log_verify(unsigned long *bmap)
value_ptr = host_test_mem + page * host_page_size;
/* If this is a special page that we were tracking... */
- if (test_and_clear_bit(page, host_bmap_track)) {
+ if (test_and_clear_bit_le(page, host_bmap_track)) {
host_track_next_count++;
- TEST_ASSERT(test_bit(page, bmap),
+ TEST_ASSERT(test_bit_le(page, bmap),
"Page %"PRIu64" should have its dirty bit "
"set in this iteration but it is missing",
page);
[PATCH v2 1/3] KVM: selftests: Split ucall.c into architecture specific files
The way we exit from a guest to userspace is very specific to the
architecture: On x86, we use PIO, on aarch64 we are using MMIO and on
s390x we're going to use an instruction instead. The possibility to
select a type via the ucall_type_t enum is currently also completely
unused, so the code in ucall.c currently looks more complex than
required. Let's split this up into architecture specific ucall.c
files instead, so we can get rid of the #ifdefs and the unnecessary
ucall_type_t handling.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 6 +-
tools/testing/selftests/kvm/dirty_log_test.c | 2 +-
.../testing/selftests/kvm/include/kvm_util.h | 8 +-
.../testing/selftests/kvm/lib/aarch64/ucall.c | 112 +
tools/testing/selftests/kvm/lib/ucall.c | 157 --
.../testing/selftests/kvm/lib/x86_64/ucall.c | 56 +++
6 files changed, 173 insertions(+), 168 deletions(-)
create mode 100644 tools/testing/selftests/kvm/lib/aarch64/ucall.c
delete mode 100644 tools/testing/selftests/kvm/lib/ucall.c
create mode 100644 tools/testing/selftests/kvm/lib/x86_64/ucall.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index ba7849751989..a51e3b83df40 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -7,9 +7,9 @@ top_srcdir = ../../../..
KSFT_KHDR_INSTALL := 1
UNAME_M := $(shell uname -m)
-LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/ucall.c
lib/sparsebit.c
-LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c
-LIBKVM_aarch64 = lib/aarch64/processor.c
+LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c
+LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/ucall.c
+LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
LIBKVM_s390x = lib/s390x/processor.c
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c
b/tools/testing/selftests/kvm/dirty_log_test.c
index ceb52b952637..5d5ae1be4984 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -337,7 +337,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long
iterations,
vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
#endif
#ifdef __aarch64__
- ucall_init(vm, UCALL_MMIO, NULL);
+ ucall_init(vm, NULL);
#endif
/* Export the shared variables to the guest */
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index e0e66b115ef2..5463b7896a0a 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -165,12 +165,6 @@ int vm_create_device(struct kvm_vm *vm, struct
kvm_create_device *cd);
memcpy(&(g), _p, sizeof(g));\
})
-/* ucall implementation types */
-typedef enum {
- UCALL_PIO,
- UCALL_MMIO,
-} ucall_type_t;
-
/* Common ucalls */
enum {
UCALL_NONE,
@@ -186,7 +180,7 @@ struct ucall {
uint64_t args[UCALL_MAX_ARGS];
};
-void ucall_init(struct kvm_vm *vm, ucall_type_t type, void *arg);
+void ucall_init(struct kvm_vm *vm, void *arg);
void ucall_uninit(struct kvm_vm *vm);
void ucall(uint64_t cmd, int nargs, ...);
uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc);
diff --git a/tools/testing/selftests/kvm/lib/aarch64/ucall.c
b/tools/testing/selftests/kvm/lib/aarch64/ucall.c
new file mode 100644
index ..f69f951a48c0
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/aarch64/ucall.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ucall support. A ucall is a "hypercall to userspace".
+ *
+ * Copyright (C) 2018, Red Hat, Inc.
+ */
+#include "kvm_util.h"
+#include "kvm_util_internal.h"
+
+static vm_vaddr_t *ucall_exit_mmio_addr;
+
+static bool ucall_mmio_init(struct kvm_vm *vm, vm_paddr_t gpa)
+{
+ if (kvm_userspace_memory_region_find(vm, gpa, gpa + 1))
+ return false;
+
+ virt_pg_map(vm, gpa, gpa, 0);
+
+ ucall_exit_mmio_addr = (vm_vaddr_t *)gpa;
+ sync_global_to_guest(vm, ucall_exit_mmio_addr);
+
+ return true;
+}
+
+void ucall_init(struct kvm_vm *vm, void *arg)
+{
+ vm_paddr_t gpa, start, end, step, offset;
+ unsigned int bits;
+ bool ret;
+
+ if (arg) {
+ gpa = (vm_paddr_t)arg;
+ ret = ucall_mmio_init(vm, gpa);
+ TEST_ASSERT(ret, "Can't set ucall mmio address to %lx", gpa);
+ return;
+ }
+
+ /*
+* Find an address within the allowed physical and virtual address
+* spaces, that does _not_ have a KVM memory region associated with
+* it. Identity mapping an address like this allows the guest to
+* access it, but as KVM doesn't know what to do with
[PATCH v2 2/3] KVM: selftests: Implement ucall() for s390x
On s390x, we can neither exit via PIO nor MMIO, but have to use an
instruction like DIAGNOSE. Now that ucall() is implemented, we can
use it in the sync_reg_test on s390x, too.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 2 +-
tools/testing/selftests/kvm/lib/s390x/ucall.c | 56 +++
.../selftests/kvm/s390x/sync_regs_test.c | 6 +-
3 files changed, 61 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/kvm/lib/s390x/ucall.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index a51e3b83df40..75ea1ecbf85a 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -10,7 +10,7 @@ UNAME_M := $(shell uname -m)
LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c
LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/ucall.c
LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
-LIBKVM_s390x = lib/s390x/processor.c
+LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
diff --git a/tools/testing/selftests/kvm/lib/s390x/ucall.c
b/tools/testing/selftests/kvm/lib/s390x/ucall.c
new file mode 100644
index ..fd589dc9bfab
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/s390x/ucall.c
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ucall support. A ucall is a "hypercall to userspace".
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ */
+#include "kvm_util.h"
+
+void ucall_init(struct kvm_vm *vm, void *arg)
+{
+}
+
+void ucall_uninit(struct kvm_vm *vm)
+{
+}
+
+void ucall(uint64_t cmd, int nargs, ...)
+{
+ struct ucall uc = {
+ .cmd = cmd,
+ };
+ va_list va;
+ int i;
+
+ nargs = nargs <= UCALL_MAX_ARGS ? nargs : UCALL_MAX_ARGS;
+
+ va_start(va, nargs);
+ for (i = 0; i < nargs; ++i)
+ uc.args[i] = va_arg(va, uint64_t);
+ va_end(va);
+
+ /* Exit via DIAGNOSE 0x501 (normally used for breakpoints) */
+ asm volatile ("diag 0,%0,0x501" : : "a"() : "memory");
+}
+
+uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc)
+{
+ struct kvm_run *run = vcpu_state(vm, vcpu_id);
+ struct ucall ucall = {};
+
+ if (run->exit_reason == KVM_EXIT_S390_SIEIC &&
+ run->s390_sieic.icptcode == 4 &&
+ (run->s390_sieic.ipa >> 8) == 0x83 &&/* 0x83 means DIAGNOSE */
+ (run->s390_sieic.ipb >> 16) == 0x501) {
+ int reg = run->s390_sieic.ipa & 0xf;
+
+ memcpy(, addr_gva2hva(vm, run->s.regs.gprs[reg]),
+ sizeof(ucall));
+
+ vcpu_run_complete_io(vm, vcpu_id);
+ if (uc)
+ memcpy(uc, , sizeof(ucall));
+ }
+
+ return ucall.cmd;
+}
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index e85ff0d69548..bbc93094519b 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -25,9 +25,11 @@
static void guest_code(void)
{
+ register u64 stage asm("11") = 0;
+
for (;;) {
- asm volatile ("diag 0,0,0x501");
- asm volatile ("ahi 11,1");
+ GUEST_SYNC(0);
+ asm volatile ("ahi %0,1" : : "r"(stage));
}
}
--
2.21.0
Re: [PATCH 1/2] KVM: selftests: Implement ucall() for s390x
On 31/07/2019 12.28, Andrew Jones wrote:
> On Wed, Jul 31, 2019 at 11:43:16AM +0200, Thomas Huth wrote:
>> On 30/07/2019 12.48, Andrew Jones wrote:
>>> On Tue, Jul 30, 2019 at 12:01:11PM +0200, Thomas Huth wrote:
>>>> On s390x, we can neither exit via PIO nor MMIO, but have to use
>>>> an instruction like DIAGNOSE. While we're at it, rename UCALL_PIO
>>>> to UCALL_DEFAULT, since PIO only works on x86 anyway, and this
>>>> way we can re-use the "default" type for the DIAGNOSE exit on s390x.
>>>>
>>>> Now that ucall() is implemented, we can use it in the sync_reg_test
>>>> on s390x, too.
>>>>
>>>> Signed-off-by: Thomas Huth
>>>> ---
>>>> .../testing/selftests/kvm/include/kvm_util.h | 2 +-
>>>> tools/testing/selftests/kvm/lib/ucall.c | 34 +++
>>>> .../selftests/kvm/s390x/sync_regs_test.c | 6 ++--
>>>> 3 files changed, 32 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
>>>> b/tools/testing/selftests/kvm/include/kvm_util.h
>>>> index e0e66b115ef2..c37aea2e33e5 100644
>>>> --- a/tools/testing/selftests/kvm/include/kvm_util.h
>>>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>>>> @@ -167,7 +167,7 @@ int vm_create_device(struct kvm_vm *vm, struct
>>>> kvm_create_device *cd);
>>>>
>>>> /* ucall implementation types */
>>>> typedef enum {
>>>> - UCALL_PIO,
>>>> + UCALL_DEFAULT,
>>>
>>> I'd rather we keep explicit types defined; keep PIO and add DIAG. Then
>>> we can have
>>>
>>> /* Set default ucall types */
>>> #if defined(__x86_64__)
>>> ucall_type = UCALL_PIO;
>>> #elif defined(__aarch64__)
>>> ucall_type = UCALL_MMIO;
>>> ucall_requires_init = true;
>>> #elif defined(__s390x__)
>>> ucall_type = UCALL_DIAG;
>>> #endif
>>>
>>> And add an assert in get_ucall()
>>>
>>> assert(!ucall_requires_init || ucall_initialized);
>>
>> I'm not sure whether I really like that. It's yet another additional
>> #ifdef block, and yet another variable ...
>>
>> What do you think about removing the enum completely and simply code it
>> directly, without the ucall_type indirection, i.e.:
>>
>> void ucall(uint64_t cmd, int nargs, ...)
>> {
>> struct ucall uc = {
>> .cmd = cmd,
>> };
>> va_list va;
>> int i;
>>
>> nargs = nargs <= UCALL_MAX_ARGS ? nargs : UCALL_MAX_ARGS;
>>
>> va_start(va, nargs);
>> for (i = 0; i < nargs; ++i)
>> uc.args[i] = va_arg(va, uint64_t);
>> va_end(va);
>>
>> #if defined(__x86_64__)
>>
>> /* Exit via PIO */
>> asm volatile("in %[port], %%al"
>> : : [port] "d" (UCALL_PIO_PORT), "D" () : "rax");
>>
>> #elif defined(__aarch64__)
>>
>> *ucall_exit_mmio_addr = (vm_vaddr_t)
>>
>> #elif defined(__s390x__)
>>
>> /* Exit via DIAGNOSE 0x501 (normally used for breakpoints) */
>> asm volatile ("diag 0,%0,0x501" : : "a"() : "memory");
>>
>> #endif
>> }
>>
>> I think that's way less confusing than having to understand the meaning
>> of ucall_type etc. before...?
>>
>
> Sounds good to me.
Or maybe even better: Let's move this file into lib/x86_64/ and
lib/aarch64/ instead, since there is more different code between the
architectures here than common code.
Thomas
Re: [PATCH 1/2] KVM: selftests: Implement ucall() for s390x
On 30/07/2019 12.48, Andrew Jones wrote:
> On Tue, Jul 30, 2019 at 12:01:11PM +0200, Thomas Huth wrote:
>> On s390x, we can neither exit via PIO nor MMIO, but have to use
>> an instruction like DIAGNOSE. While we're at it, rename UCALL_PIO
>> to UCALL_DEFAULT, since PIO only works on x86 anyway, and this
>> way we can re-use the "default" type for the DIAGNOSE exit on s390x.
>>
>> Now that ucall() is implemented, we can use it in the sync_reg_test
>> on s390x, too.
>>
>> Signed-off-by: Thomas Huth
>> ---
>> .../testing/selftests/kvm/include/kvm_util.h | 2 +-
>> tools/testing/selftests/kvm/lib/ucall.c | 34 +++
>> .../selftests/kvm/s390x/sync_regs_test.c | 6 ++--
>> 3 files changed, 32 insertions(+), 10 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
>> b/tools/testing/selftests/kvm/include/kvm_util.h
>> index e0e66b115ef2..c37aea2e33e5 100644
>> --- a/tools/testing/selftests/kvm/include/kvm_util.h
>> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
>> @@ -167,7 +167,7 @@ int vm_create_device(struct kvm_vm *vm, struct
>> kvm_create_device *cd);
>>
>> /* ucall implementation types */
>> typedef enum {
>> -UCALL_PIO,
>> +UCALL_DEFAULT,
>
> I'd rather we keep explicit types defined; keep PIO and add DIAG. Then
> we can have
>
> /* Set default ucall types */
> #if defined(__x86_64__)
> ucall_type = UCALL_PIO;
> #elif defined(__aarch64__)
> ucall_type = UCALL_MMIO;
> ucall_requires_init = true;
> #elif defined(__s390x__)
> ucall_type = UCALL_DIAG;
> #endif
>
> And add an assert in get_ucall()
>
> assert(!ucall_requires_init || ucall_initialized);
I'm not sure whether I really like that. It's yet another additional
#ifdef block, and yet another variable ...
What do you think about removing the enum completely and simply code it
directly, without the ucall_type indirection, i.e.:
void ucall(uint64_t cmd, int nargs, ...)
{
struct ucall uc = {
.cmd = cmd,
};
va_list va;
int i;
nargs = nargs <= UCALL_MAX_ARGS ? nargs : UCALL_MAX_ARGS;
va_start(va, nargs);
for (i = 0; i < nargs; ++i)
uc.args[i] = va_arg(va, uint64_t);
va_end(va);
#if defined(__x86_64__)
/* Exit via PIO */
asm volatile("in %[port], %%al"
: : [port] "d" (UCALL_PIO_PORT), "D" () : "rax");
#elif defined(__aarch64__)
*ucall_exit_mmio_addr = (vm_vaddr_t)
#elif defined(__s390x__)
/* Exit via DIAGNOSE 0x501 (normally used for breakpoints) */
asm volatile ("diag 0,%0,0x501" : : "a"() : "memory");
#endif
}
I think that's way less confusing than having to understand the meaning
of ucall_type etc. before...?
Thomas
Re: [PATCH 2/2] KVM: selftests: Enable dirty_log_test on s390x
On 30/07/2019 12.57, Andrew Jones wrote: > On Tue, Jul 30, 2019 at 12:01:12PM +0200, Thomas Huth wrote: >> To run the dirty_log_test on s390x, we have to make sure that we >> access the dirty log bitmap with little endian byte ordering and >> we have to properly align the memslot of the guest. >> Also all dirty bits of a segment are set once on s390x when one >> of the pages of a segment are written to for the first time, so >> we have to make sure that we touch all pages during the first >> iteration to keep the test in sync here. >> >> Signed-off-by: Thomas Huth >> --- [...] >> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c >> b/tools/testing/selftests/kvm/dirty_log_test.c >> index ceb52b952637..7a1223ad0ff3 100644 >> --- a/tools/testing/selftests/kvm/dirty_log_test.c >> +++ b/tools/testing/selftests/kvm/dirty_log_test.c >> @@ -26,9 +26,22 @@ >> /* The memory slot index to track dirty pages */ >> #define TEST_MEM_SLOT_INDEX 1 >> >> +#ifdef __s390x__ >> + >> +/* >> + * On s390x, the ELF program is sometimes linked at 0x8000, so we can >> + * not use 0x4000 here without overlapping into that region. Thus let's >> + * use 0xc000 as base address there instead. >> + */ >> +#define DEFAULT_GUEST_TEST_MEM 0xc000 > > I think both x86 and aarch64 should be ok with this offset. If testing > proves it does, then we can just change it for all architecture. Ok. It seems to work on x86 - could you please check aarch64, since I don't have such a system available right now? >> +/* Dirty bitmaps are always little endian, so we need to swap on big endian >> */ >> +#if defined(__s390x__) >> +# define BITOP_LE_SWIZZLE ((BITS_PER_LONG-1) & ~0x7) >> +# define test_bit_le(nr, addr) \ >> +test_bit((nr) ^ BITOP_LE_SWIZZLE, addr) >> +# define set_bit_le(nr, addr) \ >> +set_bit((nr) ^ BITOP_LE_SWIZZLE, addr) >> +# define clear_bit_le(nr, addr) \ >> +clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr) >> +# define test_and_set_bit_le(nr, addr) \ >> +test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, addr) >> +# define test_and_clear_bit_le(nr, addr) \ >> +test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr) >> +#else >> +# define test_bit_letest_bit >> +# define set_bit_le set_bit >> +# define clear_bit_le clear_bit >> +# define test_and_set_bit_letest_and_set_bit >> +# define test_and_clear_bit_le test_and_clear_bit >> +#endif > > nit: does the formatting above look right after applying the patch? It looked ok to me, but I can add some more tabs to even make it nicer :) >> @@ -293,6 +341,10 @@ static void run_test(enum vm_guest_mode mode, unsigned >> long iterations, >> * case where the size is not aligned to 64 pages. >> */ >> guest_num_pages = (1ul << (30 - guest_page_shift)) + 16; >> +#ifdef __s390x__ >> +/* Round up to multiple of 1M (segment size) */ >> +guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL; > > We could maybe do this for all architectures as well. It's really only needed on s390x, so I think we should keep the #ifdef here. Thomas
Re: [PATCH 2/2] KVM: selftests: Enable dirty_log_test on s390x
On 30/07/2019 16.57, Christian Borntraeger wrote:
>
>
> On 30.07.19 12:01, Thomas Huth wrote:
>> To run the dirty_log_test on s390x, we have to make sure that we
>> access the dirty log bitmap with little endian byte ordering and
>> we have to properly align the memslot of the guest.
>> Also all dirty bits of a segment are set once on s390x when one
>> of the pages of a segment are written to for the first time, so
>> we have to make sure that we touch all pages during the first
>> iteration to keep the test in sync here.
>
> While this fixes the test (and the migration does work fine), it still
> means that s390x overindicates the dirty bit for sparsely populated
> 1M segments. It is just a performance issue, but maybe we should try
> to get this fixed.
I hope you don't expect me to fix this - the gmap code is really not my
turf...
> Not sure what to do here to remember us about this,
> adding this as expected fail?
There is no such thing like an expected failure in KVM selftests -
that's only available in kvm-unit-tests.
So the only option that I currently see is to add a printf("TODO: ...")
on s390x here... would that work for you?
Thomas
[PATCH 1/2] KVM: selftests: Implement ucall() for s390x
On s390x, we can neither exit via PIO nor MMIO, but have to use
an instruction like DIAGNOSE. While we're at it, rename UCALL_PIO
to UCALL_DEFAULT, since PIO only works on x86 anyway, and this
way we can re-use the "default" type for the DIAGNOSE exit on s390x.
Now that ucall() is implemented, we can use it in the sync_reg_test
on s390x, too.
Signed-off-by: Thomas Huth
---
.../testing/selftests/kvm/include/kvm_util.h | 2 +-
tools/testing/selftests/kvm/lib/ucall.c | 34 +++
.../selftests/kvm/s390x/sync_regs_test.c | 6 ++--
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index e0e66b115ef2..c37aea2e33e5 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -167,7 +167,7 @@ int vm_create_device(struct kvm_vm *vm, struct
kvm_create_device *cd);
/* ucall implementation types */
typedef enum {
- UCALL_PIO,
+ UCALL_DEFAULT,
UCALL_MMIO,
} ucall_type_t;
diff --git a/tools/testing/selftests/kvm/lib/ucall.c
b/tools/testing/selftests/kvm/lib/ucall.c
index dd9a66700f96..55534dd014dc 100644
--- a/tools/testing/selftests/kvm/lib/ucall.c
+++ b/tools/testing/selftests/kvm/lib/ucall.c
@@ -30,7 +30,7 @@ void ucall_init(struct kvm_vm *vm, ucall_type_t type, void
*arg)
ucall_type = type;
sync_global_to_guest(vm, ucall_type);
- if (type == UCALL_PIO)
+ if (type == UCALL_DEFAULT)
return;
if (type == UCALL_MMIO) {
@@ -84,11 +84,18 @@ void ucall_uninit(struct kvm_vm *vm)
sync_global_to_guest(vm, ucall_exit_mmio_addr);
}
-static void ucall_pio_exit(struct ucall *uc)
+static void ucall_default_exit(struct ucall *uc)
{
-#ifdef __x86_64__
+#if defined(__x86_64__)
+ /* Exit via PIO */
asm volatile("in %[port], %%al"
: : [port] "d" (UCALL_PIO_PORT), "D" (uc) : "rax");
+#elif defined(__s390x__)
+ /* Exit via DIAGNOSE 0x501 (normally used for breakpoints) */
+ asm volatile ("diag 0,%0,0x501" : : "a"(uc) : "memory");
+#else
+ fprintf(stderr, "No default ucall available on this architecture.\n");
+ exit(1);
#endif
}
@@ -113,8 +120,8 @@ void ucall(uint64_t cmd, int nargs, ...)
va_end(va);
switch (ucall_type) {
- case UCALL_PIO:
- ucall_pio_exit();
+ case UCALL_DEFAULT:
+ ucall_default_exit();
break;
case UCALL_MMIO:
ucall_mmio_exit();
@@ -128,15 +135,28 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id,
struct ucall *uc)
struct ucall ucall = {};
bool got_ucall = false;
-#ifdef __x86_64__
- if (ucall_type == UCALL_PIO && run->exit_reason == KVM_EXIT_IO &&
+#if defined(__x86_64__)
+ if (ucall_type == UCALL_DEFAULT && run->exit_reason == KVM_EXIT_IO &&
run->io.port == UCALL_PIO_PORT) {
struct kvm_regs regs;
vcpu_regs_get(vm, vcpu_id, );
memcpy(, addr_gva2hva(vm, (vm_vaddr_t)regs.rdi),
sizeof(ucall));
got_ucall = true;
}
+#elif defined(__s390x__)
+ if (ucall_type == UCALL_DEFAULT &&
+ run->exit_reason == KVM_EXIT_S390_SIEIC &&
+ run->s390_sieic.icptcode == 4 &&
+ (run->s390_sieic.ipa >> 8) == 0x83 &&/* 0x83 means DIAGNOSE */
+ (run->s390_sieic.ipb >> 16) == 0x501) {
+ int reg = run->s390_sieic.ipa & 0xf;
+
+ memcpy(, addr_gva2hva(vm, run->s.regs.gprs[reg]),
+ sizeof(ucall));
+ got_ucall = true;
+ }
#endif
+
if (ucall_type == UCALL_MMIO && run->exit_reason == KVM_EXIT_MMIO &&
run->mmio.phys_addr == (uint64_t)ucall_exit_mmio_addr) {
vm_vaddr_t gva;
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index e85ff0d69548..bbc93094519b 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -25,9 +25,11 @@
static void guest_code(void)
{
+ register u64 stage asm("11") = 0;
+
for (;;) {
- asm volatile ("diag 0,0,0x501");
- asm volatile ("ahi 11,1");
+ GUEST_SYNC(0);
+ asm volatile ("ahi %0,1" : : "r"(stage));
}
}
--
2.21.0
[PATCH 0/2] KVM: selftests: Enable ucall and dirty_log_test on s390x
Implement the ucall() interface on s390x to be able to use the dirty_log_test KVM selftest on s390x, too. Thomas Huth (2): KVM: selftests: Implement ucall() for s390x KVM: selftests: Enable dirty_log_test on s390x tools/testing/selftests/kvm/Makefile | 1 + tools/testing/selftests/kvm/dirty_log_test.c | 70 +-- .../testing/selftests/kvm/include/kvm_util.h | 2 +- tools/testing/selftests/kvm/lib/ucall.c | 34 +++-- .../selftests/kvm/s390x/sync_regs_test.c | 6 +- 5 files changed, 98 insertions(+), 15 deletions(-) -- 2.21.0
[PATCH 2/2] KVM: selftests: Enable dirty_log_test on s390x
To run the dirty_log_test on s390x, we have to make sure that we
access the dirty log bitmap with little endian byte ordering and
we have to properly align the memslot of the guest.
Also all dirty bits of a segment are set once on s390x when one
of the pages of a segment are written to for the first time, so
we have to make sure that we touch all pages during the first
iteration to keep the test in sync here.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/dirty_log_test.c | 70 ++--
2 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index ba7849751989..ac7e63e00fee 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -33,6 +33,7 @@ TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c
b/tools/testing/selftests/kvm/dirty_log_test.c
index ceb52b952637..7a1223ad0ff3 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -26,9 +26,22 @@
/* The memory slot index to track dirty pages */
#define TEST_MEM_SLOT_INDEX1
+#ifdef __s390x__
+
+/*
+ * On s390x, the ELF program is sometimes linked at 0x8000, so we can
+ * not use 0x4000 here without overlapping into that region. Thus let's
+ * use 0xc000 as base address there instead.
+ */
+#define DEFAULT_GUEST_TEST_MEM 0xc000
+
+#else
+
/* Default guest test memory offset, 1G */
#define DEFAULT_GUEST_TEST_MEM 0x4000
+#endif
+
/* How many pages to dirty for each guest loop */
#define TEST_PAGES_PER_LOOP1024
@@ -38,6 +51,27 @@
/* Interval for each host loop (ms) */
#define TEST_HOST_LOOP_INTERVAL10UL
+/* Dirty bitmaps are always little endian, so we need to swap on big endian */
+#if defined(__s390x__)
+# define BITOP_LE_SWIZZLE ((BITS_PER_LONG-1) & ~0x7)
+# define test_bit_le(nr, addr) \
+ test_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define set_bit_le(nr, addr) \
+ set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define clear_bit_le(nr, addr) \
+ clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define test_and_set_bit_le(nr, addr) \
+ test_and_set_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+# define test_and_clear_bit_le(nr, addr) \
+ test_and_clear_bit((nr) ^ BITOP_LE_SWIZZLE, addr)
+#else
+# define test_bit_le test_bit
+# define set_bit_leset_bit
+# define clear_bit_le clear_bit
+# define test_and_set_bit_le test_and_set_bit
+# define test_and_clear_bit_le test_and_clear_bit
+#endif
+
/*
* Guest/Host shared variables. Ensure addr_gva2hva() and/or
* sync_global_to/from_guest() are used when accessing from
@@ -69,11 +103,25 @@ static uint64_t guest_test_virt_mem =
DEFAULT_GUEST_TEST_MEM;
*/
static void guest_code(void)
{
+ uint64_t addr;
int i;
+#ifdef __s390x__
+ /*
+* On s390x, all pages of a 1M segment are initially marked as dirty
+* when a page of the segment is written to for the very first time.
+* To compensate this specialty in this test, we need to touch all
+* pages during the first iteration.
+*/
+ for (i = 0; i < guest_num_pages; i++) {
+ addr = guest_test_virt_mem + i * guest_page_size;
+ *(uint64_t *)addr = READ_ONCE(iteration);
+ }
+#endif
+
while (true) {
for (i = 0; i < TEST_PAGES_PER_LOOP; i++) {
- uint64_t addr = guest_test_virt_mem;
+ addr = guest_test_virt_mem;
addr += (READ_ONCE(random_array[i]) % guest_num_pages)
* guest_page_size;
addr &= ~(host_page_size - 1);
@@ -158,15 +206,15 @@ static void vm_dirty_log_verify(unsigned long *bmap)
value_ptr = host_test_mem + page * host_page_size;
/* If this is a special page that we were tracking... */
- if (test_and_clear_bit(page, host_bmap_track)) {
+ if (test_and_clear_bit_le(page, host_bmap_track)) {
host_track_next_count++;
- TEST_ASSERT(test_bit(page, bmap),
+ TEST_ASSERT(test_bit_le(page, bmap),
"Page %"PRIu64" should have its dirty bit "
"set in this iteration but it is missing",
page);
}
- if (test_bit(page, bmap)) {
+ if (test_bit_le(page, bmap)
[PATCH] kernel/configs: Replace GPL boilerplate code with SPDX identifier
The FSF does not reside in "675 Mass Ave, Cambridge" anymore... let's replace the old GPL boilerplate code with a proper SPDX identifier instead. Signed-off-by: Thomas Huth --- kernel/configs.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/kernel/configs.c b/kernel/configs.c index b062425ccf8d..c09ea4c995e1 100644 --- a/kernel/configs.c +++ b/kernel/configs.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* * kernel/configs.c * Echo the kernel .config file used to build the kernel @@ -6,21 +7,6 @@ * Copyright (C) 2002 Randy Dunlap * Copyright (C) 2002 Al Stone * Copyright (C) 2002 Hewlett-Packard Company - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or (at - * your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or - * NON INFRINGEMENT. See the GNU General Public License for more - * details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ #include -- 2.21.0
[RFC PATCH] KVM: PPC: Enable the kvm_create_max_vcpus selftest on ppc64
The kvm_create_max_vcpus is generic enough so that it works out of the
box on POWER, too. We just have to provide some stubs for linking the
code from kvm_util.c.
Note that you also might have to do "ulimit -n 2500" before running the
test, to avoid that it runs out of file handles for the vCPUs.
Signed-off-by: Thomas Huth
---
RFC since the stubs are a little bit ugly (does someone here like
to implement them?), and since it's a little bit annoying that
you have to raise the ulimit for this test in case the kernel provides
more vCPUs than the default ulimit...
tools/testing/selftests/kvm/Makefile | 6 +++
.../selftests/kvm/lib/powerpc/processor.c | 37 +++
2 files changed, 43 insertions(+)
create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index ba7849751989..c92dc78ff74b 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -11,6 +11,8 @@ LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c
lib/ucall.c lib/sparsebi
LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c
LIBKVM_aarch64 = lib/aarch64/processor.c
LIBKVM_s390x = lib/s390x/processor.c
+LIBKVM_ppc64 = lib/powerpc/processor.c
+LIBKVM_ppc64le = $(LIBKVM_ppc64)
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
@@ -35,6 +37,10 @@ TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
+TEST_GEN_PROGS_ppc64 += kvm_create_max_vcpus
+
+TEST_GEN_PROGS_ppc64le = $(TEST_GEN_PROGS_ppc64)
+
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/lib/powerpc/processor.c
b/tools/testing/selftests/kvm/lib/powerpc/processor.c
new file mode 100644
index ..c0b7f06e206e
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KVM selftest s390x library code - CPU-related functions
+ */
+
+#define _GNU_SOURCE
+
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+
+void virt_pgd_alloc(struct kvm_vm *vm, uint32_t memslot)
+{
+ abort();/* TODO: implement this */
+}
+
+void virt_pg_map(struct kvm_vm *vm, uint64_t gva, uint64_t gpa,
+uint32_t memslot)
+{
+ abort();/* TODO: implement this */
+}
+
+vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
+{
+ abort();/* TODO: implement this */
+
+ return -1;
+}
+
+void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
+{
+ abort();/* TODO: implement this */
+}
+
+void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t
indent)
+{
+ abort();/* TODO: implement this */
+}
--
2.21.0
[PATCH] sparc: Remove redundant copy of the LGPL-2.0
We already provide the LGPL-2.0 text in LICENSES/preferred/LGPL-2.0, so there is no need for this additional copy here. Signed-off-by: Thomas Huth --- arch/sparc/lib/COPYING.LIB | 481 - 1 file changed, 481 deletions(-) delete mode 100644 arch/sparc/lib/COPYING.LIB diff --git a/arch/sparc/lib/COPYING.LIB b/arch/sparc/lib/COPYING.LIB deleted file mode 100644 index eb685a5ec981.. --- a/arch/sparc/lib/COPYING.LIB +++ /dev/null @@ -1,481 +0,0 @@ - GNU LIBRARY GENERAL PUBLIC LICENSE - Version 2, June 1991 - - Copyright (C) 1991 Free Software Foundation, Inc. -675 Mass Ave, Cambridge, MA 02139, USA - Everyone is permitted to copy and distribute verbatim copies - of this license document, but changing it is not allowed. - -[This is the first released version of the library GPL. It is - numbered 2 because it goes with version 2 of the ordinary GPL.] - - Preamble - - The licenses for most software are designed to take away your -freedom to share and change it. By contrast, the GNU General Public -Licenses are intended to guarantee your freedom to share and change -free software--to make sure the software is free for all its users. - - This license, the Library General Public License, applies to some -specially designated Free Software Foundation software, and to any -other libraries whose authors decide to use it. You can use it for -your libraries, too. - - When we speak of free software, we are referring to freedom, not -price. Our General Public Licenses are designed to make sure that you -have the freedom to distribute copies of free software (and charge for -this service if you wish), that you receive source code or can get it -if you want it, that you can change the software or use pieces of it -in new free programs; and that you know you can do these things. - - To protect your rights, we need to make restrictions that forbid -anyone to deny you these rights or to ask you to surrender the rights. -These restrictions translate to certain responsibilities for you if -you distribute copies of the library, or if you modify it. - - For example, if you distribute copies of the library, whether gratis -or for a fee, you must give the recipients all the rights that we gave -you. You must make sure that they, too, receive or can get the source -code. If you link a program with the library, you must provide -complete object files to the recipients so that they can relink them -with the library, after making changes to the library and recompiling -it. And you must show them these terms so they know their rights. - - Our method of protecting your rights has two steps: (1) copyright -the library, and (2) offer you this license which gives you legal -permission to copy, distribute and/or modify the library. - - Also, for each distributor's protection, we want to make certain -that everyone understands that there is no warranty for this free -library. If the library is modified by someone else and passed on, we -want its recipients to know that what they have is not the original -version, so that any problems introduced by others will not reflect on -the original authors' reputations. - - Finally, any free program is threatened constantly by software -patents. We wish to avoid the danger that companies distributing free -software will individually obtain patent licenses, thus in effect -transforming the program into proprietary software. To prevent this, -we have made it clear that any patent must be licensed for everyone's -free use or not licensed at all. - - Most GNU software, including some libraries, is covered by the ordinary -GNU General Public License, which was designed for utility programs. This -license, the GNU Library General Public License, applies to certain -designated libraries. This license is quite different from the ordinary -one; be sure to read it in full, and don't assume that anything in it is -the same as in the ordinary license. - - The reason we have a separate public license for some libraries is that -they blur the distinction we usually make between modifying or adding to a -program and simply using it. Linking a program with a library, without -changing the library, is in some sense simply using the library, and is -analogous to running a utility program or application program. However, in -a textual and legal sense, the linked executable is a combined work, a -derivative of the original library, and the ordinary General Public License -treats it as such. - - Because of this blurred distinction, using the ordinary General -Public License for libraries did not effectively promote software -sharing, because most developers did not use the libraries. We -concluded that weaker conditions might promote sharing better. - - However, unrestricted linking of non-free programs would deprive the -users of those programs of all benefit from
Re: [Qemu-devel] Running linux on qemu omap
On 24/05/2019 20.59, Aaro Koskinen wrote: > Hi, > > On Fri, May 24, 2019 at 06:00:18PM +0300, Aaro Koskinen wrote: >> Please don't delete OMAP boards quite yet :) In the mainline kernel >> they are not orphaned, they frequently get tested using actual hardware, >> and QEMU would help in additional testing. I'll try to get N8x0 boot to >> work with the minimal kernel I use on real HW. > > So it was only a matter of attaching the serial console at the QEMU side > (a hackish patch at the end of the mail). Does not look too much hackish to me. Could you please send it as a proper patch to the list (with patch description and Signed-off-by line)? Thomas > > diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c > index 906b7ca22d43..52ff83ec5147 100644 > --- a/hw/arm/nseries.c > +++ b/hw/arm/nseries.c > @@ -792,6 +792,7 @@ static void n8x0_uart_setup(struct n800_s *s) > qdev_connect_gpio_out(s->mpu->gpio, N8X0_BT_WKUP_GPIO, > csrhci_pins_get(radio)[csrhci_pin_wakeup]); > > +omap_uart_attach(s->mpu->uart[2], serial_hd(0)); > omap_uart_attach(s->mpu->uart[BT_UART], radio); > } > >
Re: [PATCH 5/9] KVM: selftests: Align memory region addresses to 1M on s390x
On 24/05/2019 10.29, Christian Borntraeger wrote: > > > On 23.05.19 19:40, Andrew Jones wrote: >> On Thu, May 23, 2019 at 06:43:05PM +0200, Thomas Huth wrote: >>> On s390x, there is a constraint that memory regions have to be aligned >>> to 1M (or running the VM will fail). Introduce a new "alignment" variable >>> in the vm_userspace_mem_region_add() function which now can be used for >>> both, huge page and s390x alignment requirements. >>> >>> Signed-off-by: Thomas Huth >>> --- >>> tools/testing/selftests/kvm/lib/kvm_util.c | 21 - >>> 1 file changed, 16 insertions(+), 5 deletions(-) >>> >>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c >>> b/tools/testing/selftests/kvm/lib/kvm_util.c >>> index 08edb8436c47..656df9d5cd4d 100644 >>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >>> @@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >>> unsigned long pmem_size = 0; >>> struct userspace_mem_region *region; >>> size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size; >>> + size_t alignment; >>> >>> TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical " >>> "address not on a page boundary.\n" >>> @@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >>> TEST_ASSERT(region != NULL, "Insufficient Memory"); >>> region->mmap_size = npages * vm->page_size; >>> >>> - /* Enough memory to align up to a huge page. */ >>> +#ifdef __s390x__ >>> + /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */ >>> + alignment = 0x10; >>> +#else >>> + alignment = 1; >>> +#endif >>> + >>> if (src_type == VM_MEM_SRC_ANONYMOUS_THP) >>> - region->mmap_size += huge_page_size; >>> + alignment = huge_page_size; >> >> I guess s390x won't ever support VM_MEM_SRC_ANONYMOUS_THP? If it does, >> then we need 'alignment = max(huge_page_size, alignment)'. Actually >> that might be a nice way to write this anyway for future-proofing. > > I can do > - alignment = huge_page_size; > + alignment = max(huge_page_size, alignment); > > when applying. Yes, please, that's certainly cleaner this way. Thanks, Thomas
[PATCH 7/9] KVM: selftests: Add the sync_regs test for s390x
The test is an adaption of the same test for x86. Note that there
are some differences in the way how s390x deals with the kvm_valid_regs
in struct kvm_run, so some of the tests had to be removed. Also this
test is not using the ucall() interface on s390x yet (which would need
some work to be usable on s390x), so it simply drops out of the VM with
a diag 0x501 breakpoint instead.
Signed-off-by: Thomas Huth
---
MAINTAINERS | 1 +
tools/testing/selftests/kvm/Makefile | 2 +
.../selftests/kvm/s390x/sync_regs_test.c | 151 ++
3 files changed, 154 insertions(+)
create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c05aa32dfbbe..fe41e2e1767a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8663,6 +8663,7 @@ F:arch/s390/include/asm/gmap.h
F: arch/s390/include/asm/kvm*
F: arch/s390/kvm/
F: arch/s390/mm/gmap.c
+F: tools/testing/selftests/kvm/s390x/
F: tools/testing/selftests/kvm/*/s390x/
KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 8495670ad107..d8beb990c8f4 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -29,6 +29,8 @@ TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
+TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
new file mode 100644
index ..e85ff0d69548
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test for s390x KVM_CAP_SYNC_REGS
+ *
+ * Based on the same test for x86:
+ * Copyright (C) 2018, Google LLC.
+ *
+ * Adaptions for s390x:
+ * Copyright (C) 2019, Red Hat, Inc.
+ *
+ * Test expected behavior of the KVM_CAP_SYNC_REGS functionality.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include
+#include
+#include
+#include
+#include
+
+#include "test_util.h"
+#include "kvm_util.h"
+
+#define VCPU_ID 5
+
+static void guest_code(void)
+{
+ for (;;) {
+ asm volatile ("diag 0,0,0x501");
+ asm volatile ("ahi 11,1");
+ }
+}
+
+#define REG_COMPARE(reg) \
+ TEST_ASSERT(left->reg == right->reg, \
+ "Register " #reg \
+ " values did not match: 0x%llx, 0x%llx\n", \
+ left->reg, right->reg)
+
+static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
+{
+ int i;
+
+ for (i = 0; i < 16; i++)
+ REG_COMPARE(gprs[i]);
+}
+
+static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
+{
+ int i;
+
+ for (i = 0; i < 16; i++)
+ REG_COMPARE(acrs[i]);
+
+ for (i = 0; i < 16; i++)
+ REG_COMPARE(crs[i]);
+}
+
+#undef REG_COMPARE
+
+#define TEST_SYNC_FIELDS (KVM_SYNC_GPRS|KVM_SYNC_ACRS|KVM_SYNC_CRS)
+#define INVALID_SYNC_FIELD 0x8000
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ struct kvm_regs regs;
+ struct kvm_sregs sregs;
+ int rv, cap;
+
+ /* Tell stdout not to buffer its content */
+ setbuf(stdout, NULL);
+
+ cap = kvm_check_cap(KVM_CAP_SYNC_REGS);
+ if (!cap) {
+ fprintf(stderr, "CAP_SYNC_REGS not supported, skipping test\n");
+ exit(KSFT_SKIP);
+ }
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+
+ run = vcpu_state(vm, VCPU_ID);
+
+ /* Request and verify all valid register sets. */
+ run->kvm_valid_regs = TEST_SYNC_FIELDS;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv);
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC,
+ "Unexpected exit reason: %u (%s)\n",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+ TEST_ASSERT(run->s390_sieic.icptcode == 4 &&
+ (run->s390_sieic.ipa >> 8) == 0x83 &&
+ (run->s390_sieic.ipb >> 16) == 0x501,
+ "Unexpected interception code: ic=%u, ipa=0x%x, ipb=0x%x\n",
+ run->s390_sieic.icptcode, run->s390_sieic.ipa,
+ run->s390_sieic.ipb);
+
+ vcpu_regs_get(vm, VCPU_ID, );
+ compare_regs(, >s.regs);
+
+ vcpu_sregs_get(vm, VCPU_ID, );
+ compare_sregs(, >s.regs);
+
+ /*
[PATCH 9/9] KVM: selftests: Move kvm_create_max_vcpus test to generic code
There is nothing x86-specific in the test apart from the VM_MODE_P52V48_4K
which we can now replace with VM_MODE_DEFAULT. Thus let's move the file to
the main folder and enable it for aarch64 and s390x, too.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/Makefile | 4 +++-
.../testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index d8beb990c8f4..aef5bd1166cf 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -21,15 +21,17 @@ TEST_GEN_PROGS_x86_64 += x86_64/evmcs_test
TEST_GEN_PROGS_x86_64 += x86_64/hyperv_cpuid
TEST_GEN_PROGS_x86_64 += x86_64/vmx_close_while_nested_test
TEST_GEN_PROGS_x86_64 += x86_64/smm_test
-TEST_GEN_PROGS_x86_64 += x86_64/kvm_create_max_vcpus
TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test
+TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
TEST_GEN_PROGS_x86_64 += dirty_log_test
TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
+TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c
b/tools/testing/selftests/kvm/kvm_create_max_vcpus.c
similarity index 93%
rename from tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c
rename to tools/testing/selftests/kvm/kvm_create_max_vcpus.c
index 50e92996f918..db78ce07c416 100644
--- a/tools/testing/selftests/kvm/x86_64/kvm_create_max_vcpus.c
+++ b/tools/testing/selftests/kvm/kvm_create_max_vcpus.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0-only
/*
* kvm_create_max_vcpus
*
@@ -28,7 +29,7 @@ void test_vcpu_creation(int first_vcpu_id, int num_vcpus)
printf("Testing creating %d vCPUs, with IDs %d...%d.\n",
num_vcpus, first_vcpu_id, first_vcpu_id + num_vcpus - 1);
- vm = vm_create(VM_MODE_P52V48_4K, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+ vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
for (i = 0; i < num_vcpus; i++) {
int vcpu_id = first_vcpu_id + i;
--
2.21.0
[PATCH 8/9] KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
KVM_CAP_MAX_VCPU_ID is currently always reporting KVM_MAX_VCPU_ID on all architectures. However, on s390x, the amount of usable CPUs is determined during runtime - it is depending on the features of the machine the code is running on. Since we are using the vcpu_id as an index into the SCA structures that are defined by the hardware (see e.g. the sca_add_vcpu() function), it is not only the amount of CPUs that is limited by the hard- ware, but also the range of IDs that we can use. Thus KVM_CAP_MAX_VCPU_ID must be determined during runtime on s390x, too. So the handling of KVM_CAP_MAX_VCPU_ID has to be moved from the common code into the architecture specific code, and on s390x we have to return the same value here as for KVM_CAP_MAX_VCPUS. This problem has been discovered with the kvm_create_max_vcpus selftest. With this change applied, the selftest now passes on s390x, too. Signed-off-by: Thomas Huth --- arch/mips/kvm/mips.c | 3 +++ arch/powerpc/kvm/powerpc.c | 3 +++ arch/s390/kvm/kvm-s390.c | 1 + arch/x86/kvm/x86.c | 3 +++ virt/kvm/arm/arm.c | 3 +++ virt/kvm/kvm_main.c| 2 -- 6 files changed, 13 insertions(+), 2 deletions(-) diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c index 6d0517ac18e5..0369f26ab96d 100644 --- a/arch/mips/kvm/mips.c +++ b/arch/mips/kvm/mips.c @@ -1122,6 +1122,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_MAX_VCPUS: r = KVM_MAX_VCPUS; break; + case KVM_CAP_MAX_VCPU_ID: + r = KVM_MAX_VCPU_ID; + break; case KVM_CAP_MIPS_FPU: /* We don't handle systems with inconsistent cpu_has_fpu */ r = !!raw_cpu_has_fpu; diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 3393b166817a..aa3a678711be 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -657,6 +657,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_MAX_VCPUS: r = KVM_MAX_VCPUS; break; + case KVM_CAP_MAX_VCPU_ID: + r = KVM_MAX_VCPU_ID; + break; #ifdef CONFIG_PPC_BOOK3S_64 case KVM_CAP_PPC_GET_SMMU_INFO: r = 1; diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 8d6d75db8de6..871d2e99b156 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -539,6 +539,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) break; case KVM_CAP_NR_VCPUS: case KVM_CAP_MAX_VCPUS: + case KVM_CAP_MAX_VCPU_ID: r = KVM_S390_BSCA_CPU_SLOTS; if (!kvm_s390_use_sca_entries()) r = KVM_MAX_VCPUS; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 536b78c4af6e..09a07d6a154e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3122,6 +3122,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_MAX_VCPUS: r = KVM_MAX_VCPUS; break; + case KVM_CAP_MAX_VCPU_ID: + r = KVM_MAX_VCPU_ID; + break; case KVM_CAP_PV_MMU:/* obsolete */ r = 0; break; diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c index 90cedebaeb94..7eeebe5e9da2 100644 --- a/virt/kvm/arm/arm.c +++ b/virt/kvm/arm/arm.c @@ -224,6 +224,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_MAX_VCPUS: r = KVM_MAX_VCPUS; break; + case KVM_CAP_MAX_VCPU_ID: + r = KVM_MAX_VCPU_ID; + break; case KVM_CAP_MSI_DEVID: if (!kvm) r = -EINVAL; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f0d13d9d125d..c09259dd6286 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3146,8 +3146,6 @@ static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) case KVM_CAP_MULTI_ADDRESS_SPACE: return KVM_ADDRESS_SPACE_NUM; #endif - case KVM_CAP_MAX_VCPU_ID: - return KVM_MAX_VCPU_ID; case KVM_CAP_NR_MEMSLOTS: return KVM_USER_MEM_SLOTS; default: -- 2.21.0
[PATCH 5/9] KVM: selftests: Align memory region addresses to 1M on s390x
On s390x, there is a constraint that memory regions have to be aligned
to 1M (or running the VM will fail). Introduce a new "alignment" variable
in the vm_userspace_mem_region_add() function which now can be used for
both, huge page and s390x alignment requirements.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/lib/kvm_util.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index 08edb8436c47..656df9d5cd4d 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
unsigned long pmem_size = 0;
struct userspace_mem_region *region;
size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
+ size_t alignment;
TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical "
"address not on a page boundary.\n"
@@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
TEST_ASSERT(region != NULL, "Insufficient Memory");
region->mmap_size = npages * vm->page_size;
- /* Enough memory to align up to a huge page. */
+#ifdef __s390x__
+ /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */
+ alignment = 0x10;
+#else
+ alignment = 1;
+#endif
+
if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
- region->mmap_size += huge_page_size;
+ alignment = huge_page_size;
+
+ /* Add enough memory to align up if necessary */
+ if (alignment > 1)
+ region->mmap_size += alignment;
+
region->mmap_start = mmap(NULL, region->mmap_size,
PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS
@@ -620,9 +632,8 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
"test_malloc failed, mmap_start: %p errno: %i",
region->mmap_start, errno);
- /* Align THP allocation up to start of a huge page. */
- region->host_mem = align(region->mmap_start,
-src_type == VM_MEM_SRC_ANONYMOUS_THP ?
huge_page_size : 1);
+ /* Align host address */
+ region->host_mem = align(region->mmap_start, alignment);
/* As needed perform madvise */
if (src_type == VM_MEM_SRC_ANONYMOUS || src_type ==
VM_MEM_SRC_ANONYMOUS_THP) {
--
2.21.0
[PATCH 6/9] KVM: selftests: Add processor code for s390x
Code that takes care of basic CPU setup, page table walking, etc.
Signed-off-by: Thomas Huth
---
MAINTAINERS | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/include/s390x/processor.h | 22 ++
.../selftests/kvm/lib/s390x/processor.c | 286 ++
4 files changed, 310 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 5cfbea4ce575..c05aa32dfbbe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8663,6 +8663,7 @@ F:arch/s390/include/asm/gmap.h
F: arch/s390/include/asm/kvm*
F: arch/s390/kvm/
F: arch/s390/mm/gmap.c
+F: tools/testing/selftests/kvm/*/s390x/
KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86)
M: Paolo Bonzini
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 79c524395ebe..8495670ad107 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -9,6 +9,7 @@ UNAME_M := $(shell uname -m)
LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/ucall.c
lib/sparsebit.c
LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c
LIBKVM_aarch64 = lib/aarch64/processor.c
+LIBKVM_s390x = lib/s390x/processor.c
TEST_GEN_PROGS_x86_64 = x86_64/platform_info_test
TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test
diff --git a/tools/testing/selftests/kvm/include/s390x/processor.h
b/tools/testing/selftests/kvm/include/s390x/processor.h
new file mode 100644
index ..e0e96a5f608c
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/s390x/processor.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * s390x processor specific defines
+ */
+#ifndef SELFTEST_KVM_PROCESSOR_H
+#define SELFTEST_KVM_PROCESSOR_H
+
+/* Bits in the region/segment table entry */
+#define REGION_ENTRY_ORIGIN~0xfffUL /* region/segment table origin*/
+#define REGION_ENTRY_PROTECT 0x200/* region protection bit */
+#define REGION_ENTRY_NOEXEC0x100/* region no-execute bit */
+#define REGION_ENTRY_OFFSET0xc0 /* region table offset*/
+#define REGION_ENTRY_INVALID 0x20 /* invalid region table entry */
+#define REGION_ENTRY_TYPE 0x0c /* region/segment table type mask */
+#define REGION_ENTRY_LENGTH0x03 /* region third length*/
+
+/* Bits in the page table entry */
+#define PAGE_INVALID 0x400 /* HW invalid bit*/
+#define PAGE_PROTECT 0x200 /* HW read-only bit */
+#define PAGE_NOEXEC0x100 /* HW no-execute bit */
+
+#endif
diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c
b/tools/testing/selftests/kvm/lib/s390x/processor.c
new file mode 100644
index ..c8759445e7d3
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KVM selftest s390x library code - CPU-related functions (page tables...)
+ *
+ * Copyright (C) 2019, Red Hat, Inc.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_name */
+
+#include "processor.h"
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+
+#define KVM_GUEST_PAGE_TABLE_MIN_PADDR 0x18
+
+#define PAGES_PER_REGION 4
+
+void virt_pgd_alloc(struct kvm_vm *vm, uint32_t memslot)
+{
+ vm_paddr_t paddr;
+
+ TEST_ASSERT(vm->page_size == 4096, "Unsupported page size: 0x%x",
+ vm->page_size);
+
+ if (vm->pgd_created)
+ return;
+
+ paddr = vm_phy_pages_alloc(vm, PAGES_PER_REGION,
+ KVM_GUEST_PAGE_TABLE_MIN_PADDR, memslot);
+ memset(addr_gpa2hva(vm, paddr), 0xff, PAGES_PER_REGION * vm->page_size);
+
+ vm->pgd = paddr;
+ vm->pgd_created = true;
+}
+
+/*
+ * Allocate 4 pages for a region/segment table (ri < 4), or one page for
+ * a page table (ri == 4). Returns a suitable region/segment table entry
+ * which points to the freshly allocated pages.
+ */
+static uint64_t virt_alloc_region(struct kvm_vm *vm, int ri, uint32_t memslot)
+{
+ uint64_t taddr;
+
+ taddr = vm_phy_pages_alloc(vm, ri < 4 ? PAGES_PER_REGION : 1,
+ KVM_GUEST_PAGE_TABLE_MIN_PADDR, memslot);
+ memset(addr_gpa2hva(vm, taddr), 0xff, PAGES_PER_REGION * vm->page_size);
+
+ return (taddr & REGION_ENTRY_ORIGIN)
+ | (((4 - ri) << 2) & REGION_ENTRY_TYPE)
+ | ((ri < 4 ? (PAGES_PER_REGION - 1) : 0) & REGION_ENTRY_LENGTH);
+}
+
+/*
+ * VM Virtual Page Map
+ *
+ * Input Args:
+ * vm - Virtual Machine
+ * gva - VM Virtual Address
+ * gpa - VM Physical Address
+ * memslot - Memory region slot for new virtual translation tables
+ *
+ * Output Args: No
[PATCH 3/9] kvm: selftests: aarch64: fix default vm mode
From: Andrew Jones VM_MODE_P52V48_4K is not a valid mode for AArch64. Replace its use in vm_create_default() with a mode that works and represents a good AArch64 default. (We didn't ever see a problem with this because we don't have any unit tests using vm_create_default(), but it's good to get it fixed in advance.) Reported-by: Thomas Huth Signed-off-by: Andrew Jones --- tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index e8c42506a09d..fa6cd340137c 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -226,7 +226,7 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages, uint64_t extra_pg_pages = (extra_mem_pages / ptrs_per_4k_pte) * 2; struct kvm_vm *vm; - vm = vm_create(VM_MODE_P52V48_4K, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR); + vm = vm_create(VM_MODE_P40V48_4K, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR); kvm_vm_elf_load(vm, program_invocation_name, 0, 0); vm_vcpu_add_default(vm, vcpuid, guest_code); -- 2.21.0
[PATCH 4/9] KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
This will be required later for tests like the kvm_create_max_vcpus
test that do not use the vm_create_default() function.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/include/kvm_util.h | 6 ++
tools/testing/selftests/kvm/lib/aarch64/processor.c | 2 +-
tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index b8bf961074fe..b6eb6471e6b2 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -43,6 +43,12 @@ enum vm_guest_mode {
NUM_VM_MODES,
};
+#ifdef __aarch64__
+#define VM_MODE_DEFAULT VM_MODE_P40V48_4K
+#else
+#define VM_MODE_DEFAULT VM_MODE_P52V48_4K
+#endif
+
#define vm_guest_mode_string(m) vm_guest_mode_string[m]
extern const char * const vm_guest_mode_string[];
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c
b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index fa6cd340137c..596ccaf09cb6 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -226,7 +226,7 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t
extra_mem_pages,
uint64_t extra_pg_pages = (extra_mem_pages / ptrs_per_4k_pte) * 2;
struct kvm_vm *vm;
- vm = vm_create(VM_MODE_P40V48_4K, DEFAULT_GUEST_PHY_PAGES +
extra_pg_pages, O_RDWR);
+ vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES +
extra_pg_pages, O_RDWR);
kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
vm_vcpu_add_default(vm, vcpuid, guest_code);
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c
b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index dc7fae9fa424..bb38bbcefac5 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -823,7 +823,7 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t
extra_mem_pages,
uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
/* Create VM */
- vm = vm_create(VM_MODE_P52V48_4K,
+ vm = vm_create(VM_MODE_DEFAULT,
DEFAULT_GUEST_PHY_PAGES + extra_pg_pages,
O_RDWR);
--
2.21.0
[PATCH 1/9] KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86 guard
struct kvm_nested_state is only available on x86 so far. To be able
to compile the code on other architectures as well, we need to wrap
the related code with #ifdefs.
Reviewed-by: Andrew Jones
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
tools/testing/selftests/kvm/lib/kvm_util.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index 8c6b9619797d..a5a4b28f14d8 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -118,10 +118,12 @@ void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
+#ifdef __x86_64__
void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state);
int vcpu_nested_state_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state, bool ignore_error);
+#endif
const char *exit_reason_str(unsigned int exit_reason);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index e9113857f44e..ba1359ac504f 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1250,6 +1250,7 @@ void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
ret, errno);
}
+#ifdef __x86_64__
void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state)
{
@@ -1281,6 +1282,7 @@ int vcpu_nested_state_set(struct kvm_vm *vm, uint32_t
vcpuid,
return ret;
}
+#endif
/*
* VM VCPU System Regs Get
--
2.21.0
[PATCH 2/9] KVM: selftests: Guard struct kvm_vcpu_events with __KVM_HAVE_VCPU_EVENTS
The struct kvm_vcpu_events code is only available on certain architectures
(arm, arm64 and x86). To be able to compile kvm_util.c also for other
architectures, we have to fence the code with __KVM_HAVE_VCPU_EVENTS.
Reviewed-by: David Hildenbrand
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
tools/testing/selftests/kvm/lib/kvm_util.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index a5a4b28f14d8..b8bf961074fe 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -114,10 +114,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_sregs *sregs);
int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_sregs *sregs);
+#ifdef __KVM_HAVE_VCPU_EVENTS
void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
+#endif
#ifdef __x86_64__
void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index ba1359ac504f..08edb8436c47 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1224,6 +1224,7 @@ void vcpu_regs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_regs *regs)
ret, errno);
}
+#ifdef __KVM_HAVE_VCPU_EVENTS
void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events)
{
@@ -1249,6 +1250,7 @@ void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
TEST_ASSERT(ret == 0, "KVM_SET_VCPU_EVENTS, failed, rc: %i errno: %i",
ret, errno);
}
+#endif
#ifdef __x86_64__
void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,
--
2.21.0
[PATCH v1 0/9] KVM selftests for s390x
This patch series enables the KVM selftests for s390x. As a first
test, the sync_regs from x86 has been adapted to s390x, and after
a fix for KVM_CAP_MAX_VCPU_ID on s390x, the kvm_create_max_vcpus
is now enabled here, too.
Please note that the ucall() interface is not used yet - since
s390x neither has PIO nor MMIO, this needs some more work first
before it becomes usable (we likely should use a DIAG hypercall
here, which is what the sync_reg test is currently using, too...
I started working on that topic, but did not finish that work
yet, so I decided to not include it yet).
RFC -> v1:
- Rebase, needed to add the first patch for vcpu_nested_state_get/set
- Added patch to introduce VM_MODE_DEFAULT macro
- Improved/cleaned up the code in processor.c
- Added patch to fix KVM_CAP_MAX_VCPU_ID on s390x
- Added patch to enable the kvm_create_max_vcpus on s390x and aarch64
Andrew Jones (1):
kvm: selftests: aarch64: fix default vm mode
Thomas Huth (8):
KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86
guard
KVM: selftests: Guard struct kvm_vcpu_events with
__KVM_HAVE_VCPU_EVENTS
KVM: selftests: Introduce a VM_MODE_DEFAULT macro for the default bits
KVM: selftests: Align memory region addresses to 1M on s390x
KVM: selftests: Add processor code for s390x
KVM: selftests: Add the sync_regs test for s390x
KVM: s390: Do not report unusabled IDs via KVM_CAP_MAX_VCPU_ID
KVM: selftests: Move kvm_create_max_vcpus test to generic code
MAINTAINERS | 2 +
arch/mips/kvm/mips.c | 3 +
arch/powerpc/kvm/powerpc.c| 3 +
arch/s390/kvm/kvm-s390.c | 1 +
arch/x86/kvm/x86.c| 3 +
tools/testing/selftests/kvm/Makefile | 7 +-
.../testing/selftests/kvm/include/kvm_util.h | 10 +
.../selftests/kvm/include/s390x/processor.h | 22 ++
.../kvm/{x86_64 => }/kvm_create_max_vcpus.c | 3 +-
.../selftests/kvm/lib/aarch64/processor.c | 2 +-
tools/testing/selftests/kvm/lib/kvm_util.c| 25 +-
.../selftests/kvm/lib/s390x/processor.c | 286 ++
.../selftests/kvm/lib/x86_64/processor.c | 2 +-
.../selftests/kvm/s390x/sync_regs_test.c | 151 +
virt/kvm/arm/arm.c| 3 +
virt/kvm/kvm_main.c | 2 -
16 files changed, 514 insertions(+), 11 deletions(-)
create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
rename tools/testing/selftests/kvm/{x86_64 => }/kvm_create_max_vcpus.c (93%)
create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
--
2.21.0
Re: [Qemu-devel] Running linux on qemu omap
On 22/05/2019 20.19, Aaro Koskinen wrote: > Hi, > > On Wed, May 22, 2019 at 11:33:41AM +0200, Corentin Labbe wrote: >> qemu-system-arm -M help |grep OMAP >> cheetah Palm Tungsten|E aka. Cheetah PDA (OMAP310) >> n800 Nokia N800 tablet aka. RX-34 (OMAP2420) >> n810 Nokia N810 tablet aka. RX-44 (OMAP2420) >> sx1 Siemens SX1 (OMAP310) V2 >> sx1-v1 Siemens SX1 (OMAP310) V1 >> The maximum I can get with omap1_defconfig is qemu-system-arm -kernel zImage -nographic -machine cheetah -append 'root=/dev/ram0 console=ttyO0' Uncompressing Linux... done, booting the kernel. then nothing more. > > With N800/N810 omap2plus_defconfig should be used instead. However, > I don't think that works either (but haven't tried recently). Also with > N800/N810 you need to append the DTB file to the kernel image. FWIW, Philippe recently posted a mail how to run older kernels on n810: https://www.mail-archive.com/qemu-devel@nongnu.org/msg610653.html HTH, Thomas
Re: [RFC PATCH 4/4] KVM: selftests: Add the sync_regs test for s390x
On 23/05/2019 12.56, Andrew Jones wrote: > On Thu, May 16, 2019 at 01:12:53PM +0200, Thomas Huth wrote: >> The test is an adaption of the same test for x86. Note that there >> are some differences in the way how s390x deals with the kvm_valid_regs >> in struct kvm_run, so some of the tests had to be removed. Also this >> test is not using the ucall() interface on s390x yet (which would need >> some work to be usable on s390x), so it simply drops out of the VM with >> a diag 0x501 breakpoint instead. >> >> Signed-off-by: Thomas Huth >> --- >> MAINTAINERS | 1 + >> tools/testing/selftests/kvm/Makefile | 2 + >> .../selftests/kvm/s390x/sync_regs_test.c | 151 ++ >> 3 files changed, 154 insertions(+) >> create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 514d1f88ee26..68f76ee9e821 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -8645,6 +8645,7 @@ F: arch/s390/include/asm/gmap.h >> F: arch/s390/include/asm/kvm* >> F: arch/s390/kvm/ >> F: arch/s390/mm/gmap.c >> +F: tools/testing/selftests/kvm/s390x/ >> F: tools/testing/selftests/kvm/*/s390x/ > > Do we need these lines added? We have tools/testing/selftests/kvm/ in the > common KVM section already. If we do want to specify them specifically, > then I guess we need x86 and arm MAINTAINERS updates as well. I think they are helpful in the sense that the s390x maintainers get CC:-ed on related patches as well, and if I've got Christian right, he's interested in getting informed here. For Arm related patches, I guess you should ask the Arm maintainers first. For x86, it does not really matter, since the maintainers are the same. Thomas
[PATCH] KVM: selftests: Wrap vcpu_nested_state_get/set functions with x86 guard
struct kvm_nested_state is only available on x86 so far. To be able
to compile the code on other architectures as well, we need to wrap
the related code with #ifdefs.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
tools/testing/selftests/kvm/lib/kvm_util.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index 8c6b9619797d..a5a4b28f14d8 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -118,10 +118,12 @@ void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
+#ifdef __x86_64__
void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state);
int vcpu_nested_state_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state, bool ignore_error);
+#endif
const char *exit_reason_str(unsigned int exit_reason);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index cf62de377310..633b22df46a4 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1248,6 +1248,7 @@ void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
ret, errno);
}
+#ifdef __x86_64__
void vcpu_nested_state_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_nested_state *state)
{
@@ -1279,6 +1280,7 @@ int vcpu_nested_state_set(struct kvm_vm *vm, uint32_t
vcpuid,
return ret;
}
+#endif
/*
* VM VCPU System Regs Get
--
2.21.0
Re: [PATCH] kvm: selftests: avoid type punning
On 20/05/2019 15.38, Paolo Bonzini wrote:
> Avoid warnings from -Wstrict-aliasing by using memcpy.
>
> Signed-off-by: Paolo Bonzini
> ---
> tools/testing/selftests/kvm/lib/ucall.c| 2 +-
> tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/ucall.c
> b/tools/testing/selftests/kvm/lib/ucall.c
> index a2ab38be2f47..b701a01cfcb6 100644
> --- a/tools/testing/selftests/kvm/lib/ucall.c
> +++ b/tools/testing/selftests/kvm/lib/ucall.c
> @@ -142,7 +142,7 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id,
> struct ucall *uc)
> vm_vaddr_t gva;
> TEST_ASSERT(run->mmio.is_write && run->mmio.len == 8,
> "Unexpected ucall exit mmio address access");
> - gva = *(vm_vaddr_t *)run->mmio.data;
> + memcpy(, run->mmio.data, sizeof(gva));
> memcpy(uc, addr_gva2hva(vm, gva), sizeof(*uc));
> }
>
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> index 61a2163cf9f1..9d62e2c7e024 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> @@ -75,7 +75,7 @@ void set_revision_id_for_vmcs12(struct kvm_nested_state
> *state,
> u32 vmcs12_revision)
> {
> /* Set revision_id in vmcs12 to vmcs12_revision. */
> - *(u32 *)(state->data) = vmcs12_revision;
> + memcpy(state->data, _revision, sizeof(u32));
> }
>
> void set_default_state(struct kvm_nested_state *state)
>
Reviewed-by: Thomas Huth
Re: [RFC PATCH 0/4] KVM selftests for s390x
On 20/05/2019 13.20, Paolo Bonzini wrote: > On 16/05/19 13:12, Thomas Huth wrote: >> This patch series enables the KVM selftests for s390x. As a first >> test, the sync_regs from x86 has been adapted to s390x. >> >> Please note that the ucall() interface is not used yet - since >> s390x neither has PIO nor MMIO, this needs some more work first >> before it becomes usable (we likely should use a DIAG hypercall >> here, which is what the sync_reg test is currently using, too...). > > No objections at all, though it would be like to have ucall plumbed in > from the beginning. I'm still looking at the ucall interface ... what I don't quite get yet is the question why the ucall_type there is selectable during runtime? Are there plans to have test that could either use UCALL_PIO or UCALL_MMIO? If not, what about moving ucall_init() and ucall() to architecture specific code in tools/testing/selftests/kvm/lib/aarch64/ and tools/testing/selftests/kvm/lib/x86_64 instead, and to remove the ucall_type stuff again (so that x86 is hard-wired to PIO and aarch64 is hard-wired to MMIO)? ... then I could add a DIAG-based ucall on s390x more easily, I think. Thomas
[PATCH] KVM: selftests: Remove duplicated TEST_ASSERT in hyperv_cpuid.c
The check for entry->index == 0 is done twice. One time should be sufficient. Suggested-by: Vitaly Kuznetsov Signed-off-by: Thomas Huth --- Vitaly already noticed this in his review to the "Fix a condition in test_hv_cpuid()" patch a couple of days ago, but so far I haven't seen any patch yet on the list that fixes this ... if I missed it instead, please simply ignore this patch. tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c index 9a21e912097c..8bdf1e7da6cc 100644 --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c @@ -52,9 +52,6 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries, TEST_ASSERT(entry->index == 0, ".index field should be zero"); - TEST_ASSERT(entry->index == 0, - ".index field should be zero"); - TEST_ASSERT(entry->flags == 0, ".flags field should be zero"); -- 2.21.0
Re: [RFC PATCH 1/4] KVM: selftests: Guard struct kvm_vcpu_events with __KVM_HAVE_VCPU_EVENTS
On 20/05/2019 09.12, Christian Borntraeger wrote: > > On 16.05.19 13:12, Thomas Huth wrote: >> The struct kvm_vcpu_events code is only available on certain architectures >> (arm, arm64 and x86). To be able to compile kvm_util.c also for other >> architectures, we've got to fence the code with __KVM_HAVE_VCPU_EVENTS. >> >> Signed-off-by: Thomas Huth > Acked-by: Christian Borntraeger > > According to the MAINTAINERS patches, you want me to pick these patches. > Correct? That would be nice, yes. But if you don't want to be responsible for s390x-related KVM selftest patches, please let me know, then I'll drop these hunks from the patches again. Thomas
Re: [PATCH v2] KVM: selftests: Compile code with warnings enabled
On 17/05/2019 11.41, Vitaly Kuznetsov wrote: > Peter Xu writes: > >> On Fri, May 17, 2019 at 11:04:45AM +0200, Thomas Huth wrote: >>> So far the KVM selftests are compiled without any compiler warnings >>> enabled. That's quite bad, since we miss a lot of possible bugs this >>> way. Let's enable at least "-Wall" and some other useful warning flags >>> now, and fix at least the trivial problems in the code (like unused >>> variables). >>> >>> Signed-off-by: Thomas Huth >>> --- >>> v2: >>> - Rebased to kvm/queue >>> - Fix warnings in state_test.c and evmcs_test.c, too >> >> I still see these warnings (probably because the hyperv_cpuid.c is a >> new test): >> >> In file included from x86_64/hyperv_cpuid.c:18: >> x86_64/hyperv_cpuid.c: In function ‘test_hv_cpuid’: >> x86_64/hyperv_cpuid.c:61:33: warning: suggest parentheses around comparison >> in operand of ‘==’ [-Wparentheses] >>TEST_ASSERT(entry->padding[0] == entry->padding[1] >>~~^~~~ >> include/test_util.h:32:15: note: in definition of macro ‘TEST_ASSERT’ >> test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__) >>^ >> x86_64/hyperv_cpuid.c:62:8: warning: suggest parentheses around comparison >> in operand of ‘==’ [-Wparentheses] >>TEST_ASSERT(entry->padding[0] == entry->padding[1] >>~~ >> == entry->padding[2] == 0, >> ^~~~ >> include/test_util.h:32:15: note: in definition of macro ‘TEST_ASSERT’ >> test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__) > > There's a fix from Dan Carpenter on the list: > https://marc.info/?l=kernel-janitors=155783012012532=2 Right, that fix should preferably be committed first. >> x86_64/hyperv_cpuid.c: In function ‘kvm_get_supported_hv_cpuid’: >> x86_64/hyperv_cpuid.c:93:6: warning: unused variable ‘ret’ >> [-Wunused-variable] >> int ret; >> ^~~ ... but I obviously missed this one here again :-/ There are also these compiler warnings left: lib/ucall.c: In function ‘get_ucall’: lib/ucall.c:145:3: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] gva = *(vm_vaddr_t *)run->mmio.data; x86_64/vmx_set_nested_state_test.c: In function ‘set_revision_id_for_vmcs12’: x86_64/vmx_set_nested_state_test.c:78:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] *(u32 *)(state->data) = vmcs12_revision; ... how do we want to handle such spots in the kvm selftest code? Compile with -fno-strict-aliasing? Or fix it with type-punning through unions? Thomas
[PATCH v2] KVM: selftests: Compile code with warnings enabled
So far the KVM selftests are compiled without any compiler warnings
enabled. That's quite bad, since we miss a lot of possible bugs this
way. Let's enable at least "-Wall" and some other useful warning flags
now, and fix at least the trivial problems in the code (like unused
variables).
Signed-off-by: Thomas Huth
---
v2:
- Rebased to kvm/queue
- Fix warnings in state_test.c and evmcs_test.c, too
tools/testing/selftests/kvm/Makefile | 4 +++-
tools/testing/selftests/kvm/dirty_log_test.c | 6 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 3 ---
tools/testing/selftests/kvm/lib/x86_64/processor.c | 4 +---
tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c | 1 +
tools/testing/selftests/kvm/x86_64/evmcs_test.c| 7 +--
tools/testing/selftests/kvm/x86_64/platform_info_test.c| 1 -
tools/testing/selftests/kvm/x86_64/smm_test.c | 3 +--
tools/testing/selftests/kvm/x86_64/state_test.c| 7 +--
.../selftests/kvm/x86_64/vmx_close_while_nested_test.c | 5 +
tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c | 5 ++---
11 files changed, 16 insertions(+), 30 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 79c524395ebe..d113eaf2d570 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -34,7 +34,9 @@ LIBKVM += $(LIBKVM_$(UNAME_M))
INSTALL_HDR_PATH = $(top_srcdir)/usr
LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
-CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE
-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(page_size;
@@ -1334,7 +1332,6 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_sregs *sregs)
int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs
*sregs)
{
struct vcpu *vcpu = vcpu_find(vm, vcpuid);
- int ret;
TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid);
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c
b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index dc7fae9fa424..21f3040d90cb 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -229,8 +229,6 @@ void sregs_dump(FILE *stream, struct kvm_sregs *sregs,
void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot)
{
- int rc;
-
TEST_ASSERT(vm->mode == VM_MODE_P52V48_4K, "Attempt to use "
"unknown or unsupported guest mode, mode: 0x%x", vm->mode);
@@ -549,7 +547,6 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
struct pageDirectoryPointerEntry *pdpe;
struct pageDirectoryEntry *pde;
struct pageTableEntry *pte;
- void *hva;
TEST_ASSERT(vm->mode == VM_MODE_P52V48_4K, "Attempt to use "
"unknown or unsupported guest mode, mode: 0x%x", vm->mode);
@@ -582,6 +579,7 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
unmapped_gva:
TEST_ASSERT(false, "No mapping for vm virtual address, "
"gva: 0x%lx", gva);
+ exit(EXIT_FAILURE);
}
static void kvm_setup_gdt(struct kvm_vm *vm, struct kvm_dtable *dt, int
gdt_memslot,
diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
index 7c2c4d4055a8..63cc9c3f5ab6 100644
--- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
@@ -87,6 +87,7 @@ int main(int argc, char *argv[])
while (1) {
rc = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rc == 0, "vcpu_run failed: %d\n", rc);
TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
"Unexpected exit reason: %u (%s),\n",
run->exit_reason,
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index 36669684eca5..b38260e29775 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -19,8 +19,6 @@
#define VCPU_ID5
-static bool have_nested_state;
-
void l2_guest_code(void)
{
GUEST_SYNC(6);
@@ -73,7 +71,6 @@ void guest_code(struct vmx_pages *vmx_pages)
int main(int argc, char *argv[])
{
- struct vmx_pages *vmx_pages = NULL;
vm_vaddr_t vmx_pages_gva = 0;
struct kvm_regs regs1, regs2;
@@ -88,8 +85,6 @@ int main(int argc, char *argv[])
.args[0] = (unsigned long)_ver
};
- struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
-
/* Create VM */
vm = vm_create_defau
Re: [PATCH] KVM: selftests: Compile code with warnings enabled
On 17/05/2019 04.45, Peter Xu wrote: > Hi, Thomas, > > On Thu, May 16, 2019 at 03:02:57PM +0200, Thomas Huth wrote: >> So far the KVM selftests are compiled without any compiler warnings >> enabled. That's quite bad, since we miss a lot of possible bugs this >> way. Let's enable at least "-Wall" and some other useful warning flags >> now. >> >> Signed-off-by: Thomas Huth >> --- >> This patch fixes most of the warnings in the x86 code already - but >> for some warnings, I was not quite sure (e.g. about the need for the >> kvm_get_supported_cpuid_entry(1) in some tests), so I did not touch > > If you mean the two calls in state_test and evmcs_test I would agree > they should be dropped directly. Yes, that were the once I had in mind. I'll drop them in v2. > Just to mention that the patch may not apply cleanly to kvm/queue now > probably because the dirty-log-test.c touchup recently, so may need a > rebase. Otherwise it looks nice at least to me to have these checks. Ok, thanks for the hint, I'll rebase and send a v2. Thomas
[PATCH] KVM: selftests: Compile code with warnings enabled
So far the KVM selftests are compiled without any compiler warnings
enabled. That's quite bad, since we miss a lot of possible bugs this
way. Let's enable at least "-Wall" and some other useful warning flags
now.
Signed-off-by: Thomas Huth
---
This patch fixes most of the warnings in the x86 code already - but
for some warnings, I was not quite sure (e.g. about the need for the
kvm_get_supported_cpuid_entry(1) in some tests), so I did not touch
that code yet. I also did not check aarch64 yet. I'd be glad if
someone who knows these parts of the code could have a look at the
warnings there.
tools/testing/selftests/kvm/Makefile| 4 +++-
tools/testing/selftests/kvm/dirty_log_test.c| 6 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 3 ---
tools/testing/selftests/kvm/lib/x86_64/processor.c | 4 +---
tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c| 1 +
tools/testing/selftests/kvm/x86_64/platform_info_test.c | 1 -
tools/testing/selftests/kvm/x86_64/smm_test.c | 3 +--
.../selftests/kvm/x86_64/vmx_close_while_nested_test.c | 5 +
tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c| 5 ++---
9 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index f8588cca2bef..93f344bb96af 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -32,7 +32,9 @@ LIBKVM += $(LIBKVM_$(UNAME_M))
INSTALL_HDR_PATH = $(top_srcdir)/usr
LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
-CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE
-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(page_size;
@@ -1302,7 +1300,6 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_sregs *sregs)
int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs
*sregs)
{
struct vcpu *vcpu = vcpu_find(vm, vcpuid);
- int ret;
TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid);
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c
b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index dc7fae9fa424..21f3040d90cb 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -229,8 +229,6 @@ void sregs_dump(FILE *stream, struct kvm_sregs *sregs,
void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot)
{
- int rc;
-
TEST_ASSERT(vm->mode == VM_MODE_P52V48_4K, "Attempt to use "
"unknown or unsupported guest mode, mode: 0x%x", vm->mode);
@@ -549,7 +547,6 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
struct pageDirectoryPointerEntry *pdpe;
struct pageDirectoryEntry *pde;
struct pageTableEntry *pte;
- void *hva;
TEST_ASSERT(vm->mode == VM_MODE_P52V48_4K, "Attempt to use "
"unknown or unsupported guest mode, mode: 0x%x", vm->mode);
@@ -582,6 +579,7 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
unmapped_gva:
TEST_ASSERT(false, "No mapping for vm virtual address, "
"gva: 0x%lx", gva);
+ exit(EXIT_FAILURE);
}
static void kvm_setup_gdt(struct kvm_vm *vm, struct kvm_dtable *dt, int
gdt_memslot,
diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
index 7c2c4d4055a8..63cc9c3f5ab6 100644
--- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
@@ -87,6 +87,7 @@ int main(int argc, char *argv[])
while (1) {
rc = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rc == 0, "vcpu_run failed: %d\n", rc);
TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
"Unexpected exit reason: %u (%s),\n",
run->exit_reason,
diff --git a/tools/testing/selftests/kvm/x86_64/platform_info_test.c
b/tools/testing/selftests/kvm/x86_64/platform_info_test.c
index eb3e7a838cb4..40050e44ec0a 100644
--- a/tools/testing/selftests/kvm/x86_64/platform_info_test.c
+++ b/tools/testing/selftests/kvm/x86_64/platform_info_test.c
@@ -81,7 +81,6 @@ static void test_msr_platform_info_disabled(struct kvm_vm *vm)
int main(int argc, char *argv[])
{
struct kvm_vm *vm;
- struct kvm_run *state;
int rv;
uint64_t msr_platform_info;
diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c
b/tools/testing/selftests/kvm/x86_64/smm_test.c
index fb8086964d83..4daf520bada1 100644
--- a/tools/testing/selftests/kvm/x86_64/smm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smm_test.c
@@ -87,7 +87,6 @@ void gue
Re: [RFC PATCH 2/4] KVM: selftests: Align memory region addresses to 1M on s390x
On 16/05/2019 13.30, David Hildenbrand wrote: > On 16.05.19 13:12, Thomas Huth wrote: >> On s390x, there is a constraint that memory regions have to be aligned >> to 1M (or running the VM will fail). Introduce a new "alignment" variable >> in the vm_userspace_mem_region_add() function which now can be used for >> both, huge page and s390x alignment requirements. >> >> Signed-off-by: Thomas Huth >> --- >> tools/testing/selftests/kvm/lib/kvm_util.c | 21 +- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c >> b/tools/testing/selftests/kvm/lib/kvm_util.c >> index 8d63ccb93e10..64a0da6efe3d 100644 >> --- a/tools/testing/selftests/kvm/lib/kvm_util.c >> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c >> @@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >> unsigned long pmem_size = 0; >> struct userspace_mem_region *region; >> size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size; >> +size_t alignment; >> >> TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical " >> "address not on a page boundary.\n" >> @@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, >> TEST_ASSERT(region != NULL, "Insufficient Memory"); >> region->mmap_size = npages * vm->page_size; >> >> -/* Enough memory to align up to a huge page. */ >> +#ifdef __s390x__ >> +/* On s390x, the host address must be aligned to 1M (due to PGSTEs) */ >> +alignment = 0x10; > > This corresponds to huge_page_size, maybe you can exploit this fact here. > > Something like > > alignment = 1; > > /* On s390x, the host address must always be aligned to the THP size */ > #ifndef __s390x__ > if (src_type == VM_MEM_SRC_ANONYMOUS_THP) > #endif > alignment = huge_page_size; > > Maybe in a nicer fashion. Not sure. Hmm, but if I've got your explanation on IRC right, it's rather a coincidence that the huge page size matches the alignment requirements for KVM memslots, isn't it? So I think the code would look rather confusing if I'd try to shorten it this way...? Thomas
[RFC PATCH 0/4] KVM selftests for s390x
This patch series enables the KVM selftests for s390x. As a first test, the sync_regs from x86 has been adapted to s390x. Please note that the ucall() interface is not used yet - since s390x neither has PIO nor MMIO, this needs some more work first before it becomes usable (we likely should use a DIAG hypercall here, which is what the sync_reg test is currently using, too...). Thomas Huth (4): KVM: selftests: Guard struct kvm_vcpu_events with __KVM_HAVE_VCPU_EVENTS KVM: selftests: Align memory region addresses to 1M on s390x KVM: selftests: Add processor code for s390x KVM: selftests: Add the sync_regs test for s390x MAINTAINERS | 2 + tools/testing/selftests/kvm/Makefile | 3 + .../testing/selftests/kvm/include/kvm_util.h | 2 + .../selftests/kvm/include/s390x/processor.h | 22 ++ tools/testing/selftests/kvm/lib/kvm_util.c| 24 +- .../selftests/kvm/lib/s390x/processor.c | 277 ++ .../selftests/kvm/s390x/sync_regs_test.c | 151 ++ 7 files changed, 476 insertions(+), 5 deletions(-) create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c -- 2.21.0
[RFC PATCH 1/4] KVM: selftests: Guard struct kvm_vcpu_events with __KVM_HAVE_VCPU_EVENTS
The struct kvm_vcpu_events code is only available on certain architectures
(arm, arm64 and x86). To be able to compile kvm_util.c also for other
architectures, we've got to fence the code with __KVM_HAVE_VCPU_EVENTS.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/include/kvm_util.h | 2 ++
tools/testing/selftests/kvm/lib/kvm_util.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index 07b71ad9734a..1e46ab205038 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -114,10 +114,12 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_sregs *sregs);
int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_sregs *sregs);
+#ifdef __KVM_HAVE_VCPU_EVENTS
void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events);
+#endif
const char *exit_reason_str(unsigned int exit_reason);
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index 4ca96b228e46..8d63ccb93e10 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1224,6 +1224,7 @@ void vcpu_regs_set(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_regs *regs)
ret, errno);
}
+#ifdef __KVM_HAVE_VCPU_EVENTS
void vcpu_events_get(struct kvm_vm *vm, uint32_t vcpuid,
struct kvm_vcpu_events *events)
{
@@ -1249,6 +1250,7 @@ void vcpu_events_set(struct kvm_vm *vm, uint32_t vcpuid,
TEST_ASSERT(ret == 0, "KVM_SET_VCPU_EVENTS, failed, rc: %i errno: %i",
ret, errno);
}
+#endif
/*
* VM VCPU System Regs Get
--
2.21.0
[RFC PATCH 4/4] KVM: selftests: Add the sync_regs test for s390x
The test is an adaption of the same test for x86. Note that there
are some differences in the way how s390x deals with the kvm_valid_regs
in struct kvm_run, so some of the tests had to be removed. Also this
test is not using the ucall() interface on s390x yet (which would need
some work to be usable on s390x), so it simply drops out of the VM with
a diag 0x501 breakpoint instead.
Signed-off-by: Thomas Huth
---
MAINTAINERS | 1 +
tools/testing/selftests/kvm/Makefile | 2 +
.../selftests/kvm/s390x/sync_regs_test.c | 151 ++
3 files changed, 154 insertions(+)
create mode 100644 tools/testing/selftests/kvm/s390x/sync_regs_test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 514d1f88ee26..68f76ee9e821 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8645,6 +8645,7 @@ F:arch/s390/include/asm/gmap.h
F: arch/s390/include/asm/kvm*
F: arch/s390/kvm/
F: arch/s390/mm/gmap.c
+F: tools/testing/selftests/kvm/s390x/
F: tools/testing/selftests/kvm/*/s390x/
KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86)
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 690422c78fb2..128b3551dfd0 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -27,6 +27,8 @@ TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
+TEST_GEN_PROGS_s390x += s390x/sync_regs_test
+
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
new file mode 100644
index ..e85ff0d69548
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Test for s390x KVM_CAP_SYNC_REGS
+ *
+ * Based on the same test for x86:
+ * Copyright (C) 2018, Google LLC.
+ *
+ * Adaptions for s390x:
+ * Copyright (C) 2019, Red Hat, Inc.
+ *
+ * Test expected behavior of the KVM_CAP_SYNC_REGS functionality.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#include
+#include
+#include
+#include
+#include
+
+#include "test_util.h"
+#include "kvm_util.h"
+
+#define VCPU_ID 5
+
+static void guest_code(void)
+{
+ for (;;) {
+ asm volatile ("diag 0,0,0x501");
+ asm volatile ("ahi 11,1");
+ }
+}
+
+#define REG_COMPARE(reg) \
+ TEST_ASSERT(left->reg == right->reg, \
+ "Register " #reg \
+ " values did not match: 0x%llx, 0x%llx\n", \
+ left->reg, right->reg)
+
+static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
+{
+ int i;
+
+ for (i = 0; i < 16; i++)
+ REG_COMPARE(gprs[i]);
+}
+
+static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
+{
+ int i;
+
+ for (i = 0; i < 16; i++)
+ REG_COMPARE(acrs[i]);
+
+ for (i = 0; i < 16; i++)
+ REG_COMPARE(crs[i]);
+}
+
+#undef REG_COMPARE
+
+#define TEST_SYNC_FIELDS (KVM_SYNC_GPRS|KVM_SYNC_ACRS|KVM_SYNC_CRS)
+#define INVALID_SYNC_FIELD 0x8000
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vm *vm;
+ struct kvm_run *run;
+ struct kvm_regs regs;
+ struct kvm_sregs sregs;
+ int rv, cap;
+
+ /* Tell stdout not to buffer its content */
+ setbuf(stdout, NULL);
+
+ cap = kvm_check_cap(KVM_CAP_SYNC_REGS);
+ if (!cap) {
+ fprintf(stderr, "CAP_SYNC_REGS not supported, skipping test\n");
+ exit(KSFT_SKIP);
+ }
+
+ /* Create VM */
+ vm = vm_create_default(VCPU_ID, 0, guest_code);
+
+ run = vcpu_state(vm, VCPU_ID);
+
+ /* Request and verify all valid register sets. */
+ run->kvm_valid_regs = TEST_SYNC_FIELDS;
+ rv = _vcpu_run(vm, VCPU_ID);
+ TEST_ASSERT(rv == 0, "vcpu_run failed: %d\n", rv);
+ TEST_ASSERT(run->exit_reason == KVM_EXIT_S390_SIEIC,
+ "Unexpected exit reason: %u (%s)\n",
+ run->exit_reason,
+ exit_reason_str(run->exit_reason));
+ TEST_ASSERT(run->s390_sieic.icptcode == 4 &&
+ (run->s390_sieic.ipa >> 8) == 0x83 &&
+ (run->s390_sieic.ipb >> 16) == 0x501,
+ "Unexpected interception code: ic=%u, ipa=0x%x, ipb=0x%x\n",
+ run->s390_sieic.icptcode, run->s390_sieic.ipa,
+ run->s390_sieic.ipb);
+
+ vcpu_regs_get(vm, VCPU_ID, );
+ compare_regs(, >s.regs);
+
+ vcpu_sregs_get(vm, VCPU_ID, );
+ compare_sregs(, >s.regs);
+
+ /*
[RFC PATCH 3/4] KVM: selftests: Add processor code for s390x
Code that takes care of basic CPU setup, page table walking, etc.
Signed-off-by: Thomas Huth
---
MAINTAINERS | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/include/s390x/processor.h | 22 ++
.../selftests/kvm/lib/s390x/processor.c | 277 ++
4 files changed, 301 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/s390x/processor.h
create mode 100644 tools/testing/selftests/kvm/lib/s390x/processor.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ee6cf4d1010c..514d1f88ee26 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8645,6 +8645,7 @@ F:arch/s390/include/asm/gmap.h
F: arch/s390/include/asm/kvm*
F: arch/s390/kvm/
F: arch/s390/mm/gmap.c
+F: tools/testing/selftests/kvm/*/s390x/
KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86)
M: Paolo Bonzini
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index f8588cca2bef..690422c78fb2 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -9,6 +9,7 @@ UNAME_M := $(shell uname -m)
LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/ucall.c
lib/sparsebit.c
LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c
LIBKVM_aarch64 = lib/aarch64/processor.c
+LIBKVM_s390x = lib/s390x/processor.c
TEST_GEN_PROGS_x86_64 = x86_64/platform_info_test
TEST_GEN_PROGS_x86_64 += x86_64/set_sregs_test
diff --git a/tools/testing/selftests/kvm/include/s390x/processor.h
b/tools/testing/selftests/kvm/include/s390x/processor.h
new file mode 100644
index ..e0e96a5f608c
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/s390x/processor.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * s390x processor specific defines
+ */
+#ifndef SELFTEST_KVM_PROCESSOR_H
+#define SELFTEST_KVM_PROCESSOR_H
+
+/* Bits in the region/segment table entry */
+#define REGION_ENTRY_ORIGIN~0xfffUL /* region/segment table origin*/
+#define REGION_ENTRY_PROTECT 0x200/* region protection bit */
+#define REGION_ENTRY_NOEXEC0x100/* region no-execute bit */
+#define REGION_ENTRY_OFFSET0xc0 /* region table offset*/
+#define REGION_ENTRY_INVALID 0x20 /* invalid region table entry */
+#define REGION_ENTRY_TYPE 0x0c /* region/segment table type mask */
+#define REGION_ENTRY_LENGTH0x03 /* region third length*/
+
+/* Bits in the page table entry */
+#define PAGE_INVALID 0x400 /* HW invalid bit*/
+#define PAGE_PROTECT 0x200 /* HW read-only bit */
+#define PAGE_NOEXEC0x100 /* HW no-execute bit */
+
+#endif
diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c
b/tools/testing/selftests/kvm/lib/s390x/processor.c
new file mode 100644
index ..d882b66f3e24
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KVM selftest s390x library code - CPU-related functions (page tables...)
+ *
+ * Copyright (C) 2019, Red Hat, Inc.
+ */
+
+#define _GNU_SOURCE /* for program_invocation_name */
+
+#include "processor.h"
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+
+#define KVM_GUEST_PAGE_TABLE_MIN_PADDR 0x18
+
+#define PAGES_PER_REGION 4
+
+void virt_pgd_alloc(struct kvm_vm *vm, uint32_t memslot)
+{
+ vm_paddr_t paddr;
+
+ TEST_ASSERT(vm->page_size == 4096, "Unsupported page size: 0x%x",
+ vm->page_size);
+
+ if (vm->pgd_created)
+ return;
+
+ paddr = vm_phy_pages_alloc(vm, PAGES_PER_REGION,
+ KVM_GUEST_PAGE_TABLE_MIN_PADDR, memslot);
+ memset(addr_gpa2hva(vm, paddr), 0xff, PAGES_PER_REGION * vm->page_size);
+
+ vm->pgd = paddr;
+ vm->pgd_created = true;
+}
+
+static uint64_t virt_alloc_region(struct kvm_vm *vm, int ri, uint32_t memslot)
+{
+ uint64_t taddr, entry;
+
+ taddr = vm_phy_pages_alloc(vm, PAGES_PER_REGION,
+ KVM_GUEST_PAGE_TABLE_MIN_PADDR, memslot);
+ memset(addr_gpa2hva(vm, taddr), 0xff, PAGES_PER_REGION * vm->page_size);
+
+ entry = (taddr & REGION_ENTRY_ORIGIN)
+ | (((4 - ri) << 2) & REGION_ENTRY_TYPE)
+ | ((ri < 4 ? (PAGES_PER_REGION - 1) : 0) & REGION_ENTRY_LENGTH);
+
+ return entry;
+}
+
+/*
+ * VM Virtual Page Map
+ *
+ * Input Args:
+ * vm - Virtual Machine
+ * gva - VM Virtual Address
+ * gpa - VM Physical Address
+ * memslot - Memory region slot for new virtual translation tables
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Within the VM given by vm, creates a virtual translation for the page
+ * starting at vaddr to the page starting at paddr.
+ */
+void virt_pg_map(struct kvm_vm *vm, uin
[RFC PATCH 2/4] KVM: selftests: Align memory region addresses to 1M on s390x
On s390x, there is a constraint that memory regions have to be aligned
to 1M (or running the VM will fail). Introduce a new "alignment" variable
in the vm_userspace_mem_region_add() function which now can be used for
both, huge page and s390x alignment requirements.
Signed-off-by: Thomas Huth
---
tools/testing/selftests/kvm/lib/kvm_util.c | 21 +-
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index 8d63ccb93e10..64a0da6efe3d 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -559,6 +559,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
unsigned long pmem_size = 0;
struct userspace_mem_region *region;
size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
+ size_t alignment;
TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical "
"address not on a page boundary.\n"
@@ -608,9 +609,20 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
TEST_ASSERT(region != NULL, "Insufficient Memory");
region->mmap_size = npages * vm->page_size;
- /* Enough memory to align up to a huge page. */
+#ifdef __s390x__
+ /* On s390x, the host address must be aligned to 1M (due to PGSTEs) */
+ alignment = 0x10;
+#else
+ alignment = 1;
+#endif
+
if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
- region->mmap_size += huge_page_size;
+ alignment = huge_page_size;
+
+ /* Add enough memory to align up if necessary */
+ if (alignment > 1)
+ region->mmap_size += alignment;
+
region->mmap_start = mmap(NULL, region->mmap_size,
PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS
@@ -620,9 +632,8 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
"test_malloc failed, mmap_start: %p errno: %i",
region->mmap_start, errno);
- /* Align THP allocation up to start of a huge page. */
- region->host_mem = align(region->mmap_start,
-src_type == VM_MEM_SRC_ANONYMOUS_THP ?
huge_page_size : 1);
+ /* Align host address */
+ region->host_mem = align(region->mmap_start, alignment);
/* As needed perform madvise */
if (src_type == VM_MEM_SRC_ANONYMOUS || src_type ==
VM_MEM_SRC_ANONYMOUS_THP) {
--
2.21.0
[PATCH] mtd: maps: Make uclinux_ram_map static
The blackfin architecture has been removed a while ago, so there is
no more need to declare uclinux_ram_map as a global structure.
Signed-off-by: Thomas Huth
---
drivers/mtd/maps/uclinux.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/mtd/maps/uclinux.c b/drivers/mtd/maps/uclinux.c
index aef030ca8601..de4c46318abb 100644
--- a/drivers/mtd/maps/uclinux.c
+++ b/drivers/mtd/maps/uclinux.c
@@ -31,13 +31,7 @@
#define MAP_NAME "ram"
#endif
-/*
- * Blackfin uses uclinux_ram_map during startup, so it must not be static.
- * Provide a dummy declaration to make sparse happy.
- */
-extern struct map_info uclinux_ram_map;
-
-struct map_info uclinux_ram_map = {
+static struct map_info uclinux_ram_map = {
.name = MAP_NAME,
.size = 0,
};
--
2.21.0
Re: [PATCH] s390/mm: Silence compiler warning when compiling without CONFIG_PGSTE
On 08/04/2019 09.09, David Hildenbrand wrote:
> On 07.04.19 14:55, Thomas Huth wrote:
>> If CONFIG_PGSTE is not set (e.g. when compiling without KVM), GCC complains:
>>
>> CC arch/s390/mm/pgtable.o
>> arch/s390/mm/pgtable.c:413:15: warning: ‘pmd_alloc_map’ defined but not
>> used [-Wunused-function]
>> static pmd_t *pmd_alloc_map(struct mm_struct *mm, unsigned long addr)
>>^
>>
>> Wrap the function with "#ifdef CONFIG_PGSTE" to silence the warning.
>>
>> Signed-off-by: Thomas Huth
>> ---
>> arch/s390/mm/pgtable.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
>> index 8485d6dc2754..9ebd01219812 100644
>> --- a/arch/s390/mm/pgtable.c
>> +++ b/arch/s390/mm/pgtable.c
>> @@ -410,6 +410,7 @@ static inline pmd_t pmdp_flush_lazy(struct mm_struct *mm,
>> return old;
>> }
>>
>> +#ifdef CONFIG_PGSTE
>> static pmd_t *pmd_alloc_map(struct mm_struct *mm, unsigned long addr)
>> {
>> pgd_t *pgd;
>> @@ -427,6 +428,7 @@ static pmd_t *pmd_alloc_map(struct mm_struct *mm,
>> unsigned long addr)
>> pmd = pmd_alloc(mm, pud, addr);
>> return pmd;
>> }
>> +#endif
>>
>
> We could also move the function down to the functions where it is used
Yeah, I thought about that, too. Both have advantages:
- If we keep the code here, "git blame" shows a nicer history of these
lines
- If we move the code, we need less #ifdefs
I'll leave the decision to the maintainers... Martin, Heiko?
> Reviewed-by: David Hildenbrand
Thanks,
Thomas
[PATCH] s390/mm: Silence compiler warning when compiling without CONFIG_PGSTE
If CONFIG_PGSTE is not set (e.g. when compiling without KVM), GCC complains:
CC arch/s390/mm/pgtable.o
arch/s390/mm/pgtable.c:413:15: warning: ‘pmd_alloc_map’ defined but not
used [-Wunused-function]
static pmd_t *pmd_alloc_map(struct mm_struct *mm, unsigned long addr)
^
Wrap the function with "#ifdef CONFIG_PGSTE" to silence the warning.
Signed-off-by: Thomas Huth
---
arch/s390/mm/pgtable.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 8485d6dc2754..9ebd01219812 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -410,6 +410,7 @@ static inline pmd_t pmdp_flush_lazy(struct mm_struct *mm,
return old;
}
+#ifdef CONFIG_PGSTE
static pmd_t *pmd_alloc_map(struct mm_struct *mm, unsigned long addr)
{
pgd_t *pgd;
@@ -427,6 +428,7 @@ static pmd_t *pmd_alloc_map(struct mm_struct *mm, unsigned
long addr)
pmd = pmd_alloc(mm, pud, addr);
return pmd;
}
+#endif
pmd_t pmdp_xchg_direct(struct mm_struct *mm, unsigned long addr,
pmd_t *pmdp, pmd_t new)
--
2.21.0
[PATCH] MAINTAINERS: Remove deceptive "See below" from the "Odd Fixes" description
When reading the "See below" here, I'd expect to find some further description of "Odd Fixes" a little bit later in the file. However, as far as I can see, there is no further description available. Thus let's simply remove these deceptive two words. Signed-off-by: Thomas Huth --- Please double-check ... maybe I'm just to blind to see the right paragraph where this text refers to... MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 41ce5f4..f627950 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -93,7 +93,7 @@ Descriptions of section entries: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do - much other than throw the odd patch in. See below.. + much other than throw the odd patch in. Orphan: No current maintainer [but maybe you could take the role as you write your new code]. Obsolete:Old code. Something tagged obsolete generally means -- 1.8.3.1
Re: [qemu-s390x] s390 qemu boot failure in -next
On 25.06.2018 10:02, Christian Borntraeger wrote:
>
>
> On 06/25/2018 09:27 AM, Christian Borntraeger wrote:
>> Also adding QEMU.
>>
>> On 06/25/2018 09:10 AM, Christian Borntraeger wrote:
>>>
>>>
>>> On 06/22/2018 09:47 PM, Guenter Roeck wrote:
Hi,
starting with commit 's390/boot: make head.S and als.c be part of the
decompressor only' in -next, s390 immages no longer boot in qemu.
As far as I can see, the reason is that the command line is no longer
passed from qemu to the kernel, which results in a panic because the
root file system can not be mounted.
Was this change made on purpose ? If so, is there a way to get qemu
back to working ?
>>>
>>> Certainly not on purpose.
>>>
>>> Vasily, I can reproduce this with KVM and an external kernel boot of the
>>> vmlinux file (the elf file)
>>>
>>> e.g.
>>>
>>> qemu-system-s390 -enable-kvm -nographic -kernel vmlinux -append "this
>>> string no longer is command line"
>>>
>>> The compressed image (bzImage) seems to work fine though.
>>>
>>> This seems to be an unfortunate side effect of QEMUs ways to "guess" its
>>> Linux (checking for start
>>> address 0x1, which is no longer true for the vmlinux file). With the
>>> pure vmlinux elf file
>>> the load address is 0x10 as there is no unpacker.
>>>
>>> Guenter, can you check if arch/s390/boot/bzImage works for you as a
>>> workaround?
>>
>> Something like this in QEMU
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index f278036fa7..14153ce880 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -187,11 +187,13 @@ static void s390_ipl_realize(DeviceState *dev, Error
>> **errp)
>> */
>> if (pentry == KERN_IMAGE_START || pentry == 0x800) {
>> ipl->start_addr = KERN_IMAGE_START;
>> -/* Overwrite parameters in the kernel image, which are "rom" */
>> -strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
>> } else {
>> ipl->start_addr = pentry;
>> }
>> + if (ipl->cmdline) {
>> +/* If there is a command line, put it in the right place */
>> +strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
You definitely need to check for rom_ptr() != NULL first before calling
strcpy. Otherwise QEMU segfaults in case the user tried to load a kernel
to a different location.
See also:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04227.html
Thomas
Re: [qemu-s390x] s390 qemu boot failure in -next
On 25.06.2018 10:02, Christian Borntraeger wrote:
>
>
> On 06/25/2018 09:27 AM, Christian Borntraeger wrote:
>> Also adding QEMU.
>>
>> On 06/25/2018 09:10 AM, Christian Borntraeger wrote:
>>>
>>>
>>> On 06/22/2018 09:47 PM, Guenter Roeck wrote:
Hi,
starting with commit 's390/boot: make head.S and als.c be part of the
decompressor only' in -next, s390 immages no longer boot in qemu.
As far as I can see, the reason is that the command line is no longer
passed from qemu to the kernel, which results in a panic because the
root file system can not be mounted.
Was this change made on purpose ? If so, is there a way to get qemu
back to working ?
>>>
>>> Certainly not on purpose.
>>>
>>> Vasily, I can reproduce this with KVM and an external kernel boot of the
>>> vmlinux file (the elf file)
>>>
>>> e.g.
>>>
>>> qemu-system-s390 -enable-kvm -nographic -kernel vmlinux -append "this
>>> string no longer is command line"
>>>
>>> The compressed image (bzImage) seems to work fine though.
>>>
>>> This seems to be an unfortunate side effect of QEMUs ways to "guess" its
>>> Linux (checking for start
>>> address 0x1, which is no longer true for the vmlinux file). With the
>>> pure vmlinux elf file
>>> the load address is 0x10 as there is no unpacker.
>>>
>>> Guenter, can you check if arch/s390/boot/bzImage works for you as a
>>> workaround?
>>
>> Something like this in QEMU
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index f278036fa7..14153ce880 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -187,11 +187,13 @@ static void s390_ipl_realize(DeviceState *dev, Error
>> **errp)
>> */
>> if (pentry == KERN_IMAGE_START || pentry == 0x800) {
>> ipl->start_addr = KERN_IMAGE_START;
>> -/* Overwrite parameters in the kernel image, which are "rom" */
>> -strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
>> } else {
>> ipl->start_addr = pentry;
>> }
>> + if (ipl->cmdline) {
>> +/* If there is a command line, put it in the right place */
>> +strcpy(rom_ptr(KERN_PARM_AREA), ipl->cmdline);
You definitely need to check for rom_ptr() != NULL first before calling
strcpy. Otherwise QEMU segfaults in case the user tried to load a kernel
to a different location.
See also:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04227.html
Thomas
Re: [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
On 13.04.2018 16:32, Daniel Vetter wrote: > On Fri, Apr 13, 2018 at 11:40 AM, Thomas Huth <th...@redhat.com> wrote: >> By enabling the DRM code for virtio-gpu on S390, you currently also get >> all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically. >> This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it >> would be great if the DRM code could also be compiled without CONFIG_HDMI >> and CONFIG_I2C. These two patches now refactor the DRM code a little bit >> so that we can compile it also without CONFIG_HDMI and CONFIG_I2C. >> >> Thomas Huth (2): >> drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file >> drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C > > What's the benefit? Why does I2C/HDMI hurt you? Why should I be forced to compile-in subsystems that do not make any sense on this architecture? It's just completely weird to see CONFIG_I2C enabled on s390x. Thomas
Re: [PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
On 13.04.2018 16:32, Daniel Vetter wrote: > On Fri, Apr 13, 2018 at 11:40 AM, Thomas Huth wrote: >> By enabling the DRM code for virtio-gpu on S390, you currently also get >> all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically. >> This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it >> would be great if the DRM code could also be compiled without CONFIG_HDMI >> and CONFIG_I2C. These two patches now refactor the DRM code a little bit >> so that we can compile it also without CONFIG_HDMI and CONFIG_I2C. >> >> Thomas Huth (2): >> drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file >> drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C > > What's the benefit? Why does I2C/HDMI hurt you? Why should I be forced to compile-in subsystems that do not make any sense on this architecture? It's just completely weird to see CONFIG_I2C enabled on s390x. Thomas
[PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
By enabling the DRM code for virtio-gpu on S390, you currently also get all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically. This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it would be great if the DRM code could also be compiled without CONFIG_HDMI and CONFIG_I2C. These two patches now refactor the DRM code a little bit so that we can compile it also without CONFIG_HDMI and CONFIG_I2C. Thomas Huth (2): drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C drivers/gpu/drm/Kconfig | 6 +- drivers/gpu/drm/Makefile| 17 ++-- drivers/gpu/drm/drm_crtc_internal.h | 2 + drivers/gpu/drm/drm_edid.c | 173 ++ drivers/gpu/drm/drm_hdmi.c | 182 5 files changed, 206 insertions(+), 174 deletions(-) create mode 100644 drivers/gpu/drm/drm_hdmi.c -- 1.8.3.1
[PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C
Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
graphic hardware on this architecture. The drm subsystem is only
enabled here for using the virtual graphics card "virtio-gpu". So
it should be possible to compile the drm subsystem also without
CONFIG_I2C. Tweak the Makefile to only compile the affected files
with CONFIG_I2C=y and disable some I2C-related code in drm_edid.c.
Signed-off-by: Thomas Huth <th...@redhat.com>
---
drivers/gpu/drm/Kconfig| 4 ++--
drivers/gpu/drm/Makefile | 16 +---
drivers/gpu/drm/drm_edid.c | 10 +++---
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 298a518..c9df67f 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -10,8 +10,8 @@ menuconfig DRM
select DRM_PANEL_ORIENTATION_QUIRKS
select HDMI if !S390
select FB_CMDLINE
- select I2C
- select I2C_ALGOBIT
+ select I2C if !S390
+ select I2C_ALGOBIT if !S390
select DMA_SHARED_BUFFER
select SYNC_FILE
help
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 16fd8e3..4cf53ba 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -7,10 +7,9 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
drm_context.o drm_dma.o \
drm_file.o drm_gem.o drm_ioctl.o drm_irq.o \
drm_lock.o drm_memory.o drm_drv.o \
- drm_scatter.o drm_pci.o \
+ drm_scatter.o drm_pci.o drm_info.o \
drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
- drm_info.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -20,6 +19,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
drm_syncobj.o drm_lease.o
+drm-$(CONFIG_I2C) += drm_encoder_slave.o
drm-$(CONFIG_HDMI) += drm_hdmi.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
@@ -32,11 +32,13 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
-drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
- drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
- drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
- drm_simple_kms_helper.o drm_modeset_helper.o \
- drm_scdc_helper.o drm_gem_framebuffer_helper.o
+drm_kms_helper-y := drm_crtc_helper.o drm_probe_helper.o \
+ drm_plane_helper.o drm_atomic_helper.o \
+ drm_gem_framebuffer_helper.o drm_kms_helper_common.o \
+ drm_simple_kms_helper.o drm_modeset_helper.o
+
+drm_kms_helper-$(CONFIG_I2C) += drm_dp_helper.o drm_dp_mst_topology.o \
+ drm_dp_dual_mode_helper.o drm_scdc_helper.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3820763..0d0d5af 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1467,11 +1467,14 @@ bool drm_edid_is_valid(struct edid *edid)
static int
drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
{
- struct i2c_adapter *adapter = data;
- unsigned char start = block * EDID_LENGTH;
unsigned char segment = block >> 1;
unsigned char xfers = segment ? 3 : 2;
- int ret, retries = 5;
+ int ret = -1;
+
+#if IS_ENABLED(CONFIG_I2C)
+ struct i2c_adapter *adapter = data;
+ unsigned char start = block * EDID_LENGTH;
+ int retries = 5;
/*
* The core I2C driver will automatically retry the transfer if the
@@ -1512,6 +1515,7 @@ bool drm_edid_is_valid(struct edid *edid)
break;
}
} while (ret != xfers && --retries);
+#endif /* CONFIG_I2C */
return ret == xfers ? 0 : -1;
}
--
1.8.3.1
[PATCH 0/2] drm: Make it compilable without CONFIG_HDMI and CONFIG_I2C
By enabling the DRM code for virtio-gpu on S390, you currently also get all the code that is enabled by CONFIG_HDMI and CONFIG_I2C automatically. This is quite ugly, since on S390, there is no HDMI and no I2C. Thus it would be great if the DRM code could also be compiled without CONFIG_HDMI and CONFIG_I2C. These two patches now refactor the DRM code a little bit so that we can compile it also without CONFIG_HDMI and CONFIG_I2C. Thomas Huth (2): drivers/gpu/drm: Move CONFIG_HDMI-dependent code to a separate file drivers/gpu/drm: Make the DRM code compilable without CONFIG_I2C drivers/gpu/drm/Kconfig | 6 +- drivers/gpu/drm/Makefile| 17 ++-- drivers/gpu/drm/drm_crtc_internal.h | 2 + drivers/gpu/drm/drm_edid.c | 173 ++ drivers/gpu/drm/drm_hdmi.c | 182 5 files changed, 206 insertions(+), 174 deletions(-) create mode 100644 drivers/gpu/drm/drm_hdmi.c -- 1.8.3.1
[PATCH 2/2] drm: Make the DRM code compilable without CONFIG_I2C
Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
graphic hardware on this architecture. The drm subsystem is only
enabled here for using the virtual graphics card "virtio-gpu". So
it should be possible to compile the drm subsystem also without
CONFIG_I2C. Tweak the Makefile to only compile the affected files
with CONFIG_I2C=y and disable some I2C-related code in drm_edid.c.
Signed-off-by: Thomas Huth
---
drivers/gpu/drm/Kconfig| 4 ++--
drivers/gpu/drm/Makefile | 16 +---
drivers/gpu/drm/drm_edid.c | 10 +++---
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 298a518..c9df67f 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -10,8 +10,8 @@ menuconfig DRM
select DRM_PANEL_ORIENTATION_QUIRKS
select HDMI if !S390
select FB_CMDLINE
- select I2C
- select I2C_ALGOBIT
+ select I2C if !S390
+ select I2C_ALGOBIT if !S390
select DMA_SHARED_BUFFER
select SYNC_FILE
help
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 16fd8e3..4cf53ba 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -7,10 +7,9 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.o \
drm_context.o drm_dma.o \
drm_file.o drm_gem.o drm_ioctl.o drm_irq.o \
drm_lock.o drm_memory.o drm_drv.o \
- drm_scatter.o drm_pci.o \
+ drm_scatter.o drm_pci.o drm_info.o \
drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
- drm_info.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -20,6 +19,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
drm_syncobj.o drm_lease.o
+drm-$(CONFIG_I2C) += drm_encoder_slave.o
drm-$(CONFIG_HDMI) += drm_hdmi.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
@@ -32,11 +32,13 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
-drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
- drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
- drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
- drm_simple_kms_helper.o drm_modeset_helper.o \
- drm_scdc_helper.o drm_gem_framebuffer_helper.o
+drm_kms_helper-y := drm_crtc_helper.o drm_probe_helper.o \
+ drm_plane_helper.o drm_atomic_helper.o \
+ drm_gem_framebuffer_helper.o drm_kms_helper_common.o \
+ drm_simple_kms_helper.o drm_modeset_helper.o
+
+drm_kms_helper-$(CONFIG_I2C) += drm_dp_helper.o drm_dp_mst_topology.o \
+ drm_dp_dual_mode_helper.o drm_scdc_helper.o
drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3820763..0d0d5af 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1467,11 +1467,14 @@ bool drm_edid_is_valid(struct edid *edid)
static int
drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int block, size_t len)
{
- struct i2c_adapter *adapter = data;
- unsigned char start = block * EDID_LENGTH;
unsigned char segment = block >> 1;
unsigned char xfers = segment ? 3 : 2;
- int ret, retries = 5;
+ int ret = -1;
+
+#if IS_ENABLED(CONFIG_I2C)
+ struct i2c_adapter *adapter = data;
+ unsigned char start = block * EDID_LENGTH;
+ int retries = 5;
/*
* The core I2C driver will automatically retry the transfer if the
@@ -1512,6 +1515,7 @@ bool drm_edid_is_valid(struct edid *edid)
break;
}
} while (ret != xfers && --retries);
+#endif /* CONFIG_I2C */
return ret == xfers ? 0 : -1;
}
--
1.8.3.1
[PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file
Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
graphic hardware on this architecture. The drm subsystem is only
enabled here for using the virtual graphics card "virtio-gpu". So
it should be possible to compile the drm subsystem also without
CONFIG_DRM. Let's move the related code to a separate file for this.
Signed-off-by: Thomas Huth <th...@redhat.com>
---
drivers/gpu/drm/Kconfig | 2 +-
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/drm_crtc_internal.h | 2 +
drivers/gpu/drm/drm_edid.c | 163 +---
drivers/gpu/drm/drm_hdmi.c | 182
5 files changed, 188 insertions(+), 162 deletions(-)
create mode 100644 drivers/gpu/drm/drm_hdmi.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index deeefa7..298a518 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -8,7 +8,7 @@ menuconfig DRM
tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI
support)"
depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
select DRM_PANEL_ORIENTATION_QUIRKS
- select HDMI
+ select HDMI if !S390
select FB_CMDLINE
select I2C
select I2C_ALGOBIT
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff..16fd8e3 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -20,6 +20,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
drm_syncobj.o drm_lease.o
+drm-$(CONFIG_HDMI) += drm_hdmi.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 3c2b828..0804348 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -220,3 +220,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
/* drm_edid.c */
void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
+u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match);
+bool drm_valid_hdmi_vic(u8 vic);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f..3820763 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -29,7 +29,6 @@
*/
#include
#include
-#include
#include
#include
#include
@@ -3062,7 +3061,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
struct drm_display_mode *to_
*
* Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
*/
-static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
+u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
{
u8 vic;
@@ -3085,7 +3084,7 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
return 0;
}
-static bool drm_valid_hdmi_vic(u8 vic)
+bool drm_valid_hdmi_vic(u8 vic)
{
return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);
}
@@ -4817,76 +4816,6 @@ void drm_set_preferred_mode(struct drm_connector
*connector,
EXPORT_SYMBOL(drm_set_preferred_mode);
/**
- * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
- * data from a DRM display mode
- * @frame: HDMI AVI infoframe
- * @mode: DRM display mode
- * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int
-drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-const struct drm_display_mode *mode,
-bool is_hdmi2_sink)
-{
- int err;
-
- if (!frame || !mode)
- return -EINVAL;
-
- err = hdmi_avi_infoframe_init(frame);
- if (err < 0)
- return err;
-
- if (mode->flags & DRM_MODE_FLAG_DBLCLK)
- frame->pixel_repeat = 1;
-
- frame->video_code = drm_match_cea_mode(mode);
-
- /*
-* HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
-* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
-* have to make sure we dont break HDMI 1.4 sinks.
-*/
- if (!is_hdmi2_sink && frame->video_code > 64)
- frame->video_code = 0;
-
- /*
-* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
-* we should send its VIC in vendor infoframes, else send the
-* VIC in AVI infoframes. Lets check if this mode is present in
-* HDMI 1.4b 4K modes
-*/
- if (frame->video_code) {
- u8 vendor_if_vic = drm_match_hdmi_mode(mode);
- bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
- if (drm_valid_hdmi_vic(vend
[PATCH 1/2] drm: Move CONFIG_HDMI-dependent code to a separate file
Selecting CONFIG_HDMI for S390 is inappropriate - there is no real
graphic hardware on this architecture. The drm subsystem is only
enabled here for using the virtual graphics card "virtio-gpu". So
it should be possible to compile the drm subsystem also without
CONFIG_DRM. Let's move the related code to a separate file for this.
Signed-off-by: Thomas Huth
---
drivers/gpu/drm/Kconfig | 2 +-
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/drm_crtc_internal.h | 2 +
drivers/gpu/drm/drm_edid.c | 163 +---
drivers/gpu/drm/drm_hdmi.c | 182
5 files changed, 188 insertions(+), 162 deletions(-)
create mode 100644 drivers/gpu/drm/drm_hdmi.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index deeefa7..298a518 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -8,7 +8,7 @@ menuconfig DRM
tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI
support)"
depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
select DRM_PANEL_ORIENTATION_QUIRKS
- select HDMI
+ select HDMI if !S390
select FB_CMDLINE
select I2C
select I2C_ALGOBIT
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff..16fd8e3 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -20,6 +20,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \
drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
drm_syncobj.o drm_lease.o
+drm-$(CONFIG_HDMI) += drm_hdmi.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o
diff --git a/drivers/gpu/drm/drm_crtc_internal.h
b/drivers/gpu/drm/drm_crtc_internal.h
index 3c2b828..0804348 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -220,3 +220,5 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
/* drm_edid.c */
void drm_mode_fixup_1366x768(struct drm_display_mode *mode);
+u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match);
+bool drm_valid_hdmi_vic(u8 vic);
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 134069f..3820763 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -29,7 +29,6 @@
*/
#include
#include
-#include
#include
#include
#include
@@ -3062,7 +3061,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const
struct drm_display_mode *to_
*
* Returns the HDMI Video ID (VIC) of the mode or 0 if it isn't one.
*/
-static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
+u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
{
u8 vic;
@@ -3085,7 +3084,7 @@ static u8 drm_match_hdmi_mode(const struct
drm_display_mode *to_match)
return 0;
}
-static bool drm_valid_hdmi_vic(u8 vic)
+bool drm_valid_hdmi_vic(u8 vic)
{
return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes);
}
@@ -4817,76 +4816,6 @@ void drm_set_preferred_mode(struct drm_connector
*connector,
EXPORT_SYMBOL(drm_set_preferred_mode);
/**
- * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
- * data from a DRM display mode
- * @frame: HDMI AVI infoframe
- * @mode: DRM display mode
- * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int
-drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-const struct drm_display_mode *mode,
-bool is_hdmi2_sink)
-{
- int err;
-
- if (!frame || !mode)
- return -EINVAL;
-
- err = hdmi_avi_infoframe_init(frame);
- if (err < 0)
- return err;
-
- if (mode->flags & DRM_MODE_FLAG_DBLCLK)
- frame->pixel_repeat = 1;
-
- frame->video_code = drm_match_cea_mode(mode);
-
- /*
-* HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
-* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
-* have to make sure we dont break HDMI 1.4 sinks.
-*/
- if (!is_hdmi2_sink && frame->video_code > 64)
- frame->video_code = 0;
-
- /*
-* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
-* we should send its VIC in vendor infoframes, else send the
-* VIC in AVI infoframes. Lets check if this mode is present in
-* HDMI 1.4b 4K modes
-*/
- if (frame->video_code) {
- u8 vendor_if_vic = drm_match_hdmi_mode(mode);
- bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
- if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
-
Re: [PATCH v3 1/3] Kconfig : Remove HAS_IOMEM dependency for Graphics support
On 21.02.2018 12:22, Christian Borntraeger wrote:
>
>
> On 02/21/2018 12:14 PM, Thomas Huth wrote:
>> On 21.02.2018 12:09, Christian Borntraeger wrote:
>>>
>>>
>>> On 02/21/2018 11:32 AM, Cornelia Huck wrote:
>>>> On Wed, 21 Feb 2018 11:22:38 +0100
>>>> Christian Borntraeger <borntrae...@de.ibm.com> wrote:
>>>>
>>>>> On 02/21/2018 11:05 AM, Christian Borntraeger wrote:
>>>>>>
>>>>>>
>>>>>> On 02/19/2018 05:38 PM, Farhan Ali wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 02/19/2018 11:25 AM, Thomas Huth wrote:
>>>>>>>> On 19.02.2018 16:47, Farhan Ali wrote:
>>>>>>>>> The 'commit e25df1205f37 ("[S390] Kconfig: menus with depends on
>>>>>>>>> HAS_IOMEM.")'
>>>>>>>>> added the HAS_IOMEM dependecy for "Graphics support". This disabled
>>>>>>>>> the
>>>>>>>>> "Graphics support" menu for S390. But if we enable VT layer for S390,
>>>>>>>>> we would also need to enable the dummy console. So let's remove the
>>>>>>>>> HAS_IOMEM dependency.
>>>>>>>>>
>>>>>>>>> Move this dependency to sub menu items and console drivers that use
>>>>>>>>> io memory.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Farhan Ali <al...@linux.vnet.ibm.com>
>>>>>>>>> ---
>>>>>>>>> drivers/video/Kconfig | 21 +++--
>>>>>>>>> drivers/video/console/Kconfig | 4 ++--
>>>>>>>>> 2 files changed, 13 insertions(+), 12 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>>>>>>>> index 3c20af9..8f10915 100644
>>>>>>>>> --- a/drivers/video/Kconfig
>>>>>>>>> +++ b/drivers/video/Kconfig
>>>>>>>>> @@ -3,7 +3,6 @@
>>>>>>>>> #
>>>>>>>>> menu "Graphics support"
>>>>>>>>> - depends on HAS_IOMEM
>>>>>>>>> config HAVE_FB_ATMEL
>>>>>>>>> bool
>>>>>>>>> @@ -11,20 +10,22 @@ config HAVE_FB_ATMEL
>>>>>>>>> config SH_LCD_MIPI_DSI
>>>>>>>>> bool
>>>>>>>>> -source "drivers/char/agp/Kconfig"
>>>>>>>>> +if HAS_IOMEM
>>>>>>>>> + source "drivers/char/agp/Kconfig"
>>>>>>>>> -source "drivers/gpu/vga/Kconfig"
>>>>>>>>> + source "drivers/gpu/vga/Kconfig"
>>>>>>>>> -source "drivers/gpu/host1x/Kconfig"
>>>>>>>>> -source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>>>> + source "drivers/gpu/host1x/Kconfig"
>>>>>>>>> + source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>>>> -source "drivers/gpu/drm/Kconfig"
>>>>>>>>> + source "drivers/gpu/drm/Kconfig"
>>>>>>
>>>>>>
>>>>>> Hmmm, looks like that this makes it impossible to select VIRTIO_GPU -
>>>>>> need still more
>>>>>> work.
>>>>>>
>>>>> Sorry my fault. I had CONFIG_PCI disabled.
>>>>
>>>> That smells like the s390 HAS_IOMEM stuff needs more work -- I guess
>>>> that you want to enable a ccw virtio-gpu device, not a pci one, right?
>>>
>>> It is a ccw virtio-gpu. But s390 has no IOMEM without CONFIG_PCI, so you
>>> cannot
>>> select VIRTIO_GPU, which needs DRM, which need IOMEM.
>>
>> So the 'source "drivers/gpu/drm/Kconfig"' should maybe rather reside
>> outside of the "if HAS_IOMEM" path? Or does it not compile anymore that way?
>
> virtio-gpu depends on drm. So in essence it boils down to if you want
> virtio-gpu
> you also need to enable PCI, even if the actual channel is ccw.
But if you need to enable PCI to get IOMEM, I wonder why this patch here
is needed at all? The Graphics menu / VT dummy console should be
available in the config if IOMEM is enabled anyway?
Thomas
Re: [PATCH v3 1/3] Kconfig : Remove HAS_IOMEM dependency for Graphics support
On 21.02.2018 12:22, Christian Borntraeger wrote:
>
>
> On 02/21/2018 12:14 PM, Thomas Huth wrote:
>> On 21.02.2018 12:09, Christian Borntraeger wrote:
>>>
>>>
>>> On 02/21/2018 11:32 AM, Cornelia Huck wrote:
>>>> On Wed, 21 Feb 2018 11:22:38 +0100
>>>> Christian Borntraeger wrote:
>>>>
>>>>> On 02/21/2018 11:05 AM, Christian Borntraeger wrote:
>>>>>>
>>>>>>
>>>>>> On 02/19/2018 05:38 PM, Farhan Ali wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 02/19/2018 11:25 AM, Thomas Huth wrote:
>>>>>>>> On 19.02.2018 16:47, Farhan Ali wrote:
>>>>>>>>> The 'commit e25df1205f37 ("[S390] Kconfig: menus with depends on
>>>>>>>>> HAS_IOMEM.")'
>>>>>>>>> added the HAS_IOMEM dependecy for "Graphics support". This disabled
>>>>>>>>> the
>>>>>>>>> "Graphics support" menu for S390. But if we enable VT layer for S390,
>>>>>>>>> we would also need to enable the dummy console. So let's remove the
>>>>>>>>> HAS_IOMEM dependency.
>>>>>>>>>
>>>>>>>>> Move this dependency to sub menu items and console drivers that use
>>>>>>>>> io memory.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Farhan Ali
>>>>>>>>> ---
>>>>>>>>> drivers/video/Kconfig | 21 +++--
>>>>>>>>> drivers/video/console/Kconfig | 4 ++--
>>>>>>>>> 2 files changed, 13 insertions(+), 12 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>>>>>>>> index 3c20af9..8f10915 100644
>>>>>>>>> --- a/drivers/video/Kconfig
>>>>>>>>> +++ b/drivers/video/Kconfig
>>>>>>>>> @@ -3,7 +3,6 @@
>>>>>>>>> #
>>>>>>>>> menu "Graphics support"
>>>>>>>>> - depends on HAS_IOMEM
>>>>>>>>> config HAVE_FB_ATMEL
>>>>>>>>> bool
>>>>>>>>> @@ -11,20 +10,22 @@ config HAVE_FB_ATMEL
>>>>>>>>> config SH_LCD_MIPI_DSI
>>>>>>>>> bool
>>>>>>>>> -source "drivers/char/agp/Kconfig"
>>>>>>>>> +if HAS_IOMEM
>>>>>>>>> + source "drivers/char/agp/Kconfig"
>>>>>>>>> -source "drivers/gpu/vga/Kconfig"
>>>>>>>>> + source "drivers/gpu/vga/Kconfig"
>>>>>>>>> -source "drivers/gpu/host1x/Kconfig"
>>>>>>>>> -source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>>>> + source "drivers/gpu/host1x/Kconfig"
>>>>>>>>> + source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>>>> -source "drivers/gpu/drm/Kconfig"
>>>>>>>>> + source "drivers/gpu/drm/Kconfig"
>>>>>>
>>>>>>
>>>>>> Hmmm, looks like that this makes it impossible to select VIRTIO_GPU -
>>>>>> need still more
>>>>>> work.
>>>>>>
>>>>> Sorry my fault. I had CONFIG_PCI disabled.
>>>>
>>>> That smells like the s390 HAS_IOMEM stuff needs more work -- I guess
>>>> that you want to enable a ccw virtio-gpu device, not a pci one, right?
>>>
>>> It is a ccw virtio-gpu. But s390 has no IOMEM without CONFIG_PCI, so you
>>> cannot
>>> select VIRTIO_GPU, which needs DRM, which need IOMEM.
>>
>> So the 'source "drivers/gpu/drm/Kconfig"' should maybe rather reside
>> outside of the "if HAS_IOMEM" path? Or does it not compile anymore that way?
>
> virtio-gpu depends on drm. So in essence it boils down to if you want
> virtio-gpu
> you also need to enable PCI, even if the actual channel is ccw.
But if you need to enable PCI to get IOMEM, I wonder why this patch here
is needed at all? The Graphics menu / VT dummy console should be
available in the config if IOMEM is enabled anyway?
Thomas
Re: [PATCH v3 1/3] Kconfig : Remove HAS_IOMEM dependency for Graphics support
On 21.02.2018 12:09, Christian Borntraeger wrote:
>
>
> On 02/21/2018 11:32 AM, Cornelia Huck wrote:
>> On Wed, 21 Feb 2018 11:22:38 +0100
>> Christian Borntraeger <borntrae...@de.ibm.com> wrote:
>>
>>> On 02/21/2018 11:05 AM, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 02/19/2018 05:38 PM, Farhan Ali wrote:
>>>>>
>>>>>
>>>>> On 02/19/2018 11:25 AM, Thomas Huth wrote:
>>>>>> On 19.02.2018 16:47, Farhan Ali wrote:
>>>>>>> The 'commit e25df1205f37 ("[S390] Kconfig: menus with depends on
>>>>>>> HAS_IOMEM.")'
>>>>>>> added the HAS_IOMEM dependecy for "Graphics support". This disabled the
>>>>>>> "Graphics support" menu for S390. But if we enable VT layer for S390,
>>>>>>> we would also need to enable the dummy console. So let's remove the
>>>>>>> HAS_IOMEM dependency.
>>>>>>>
>>>>>>> Move this dependency to sub menu items and console drivers that use
>>>>>>> io memory.
>>>>>>>
>>>>>>> Signed-off-by: Farhan Ali <al...@linux.vnet.ibm.com>
>>>>>>> ---
>>>>>>> drivers/video/Kconfig | 21 +++--
>>>>>>> drivers/video/console/Kconfig | 4 ++--
>>>>>>> 2 files changed, 13 insertions(+), 12 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>>>>>> index 3c20af9..8f10915 100644
>>>>>>> --- a/drivers/video/Kconfig
>>>>>>> +++ b/drivers/video/Kconfig
>>>>>>> @@ -3,7 +3,6 @@
>>>>>>> #
>>>>>>> menu "Graphics support"
>>>>>>> - depends on HAS_IOMEM
>>>>>>> config HAVE_FB_ATMEL
>>>>>>> bool
>>>>>>> @@ -11,20 +10,22 @@ config HAVE_FB_ATMEL
>>>>>>> config SH_LCD_MIPI_DSI
>>>>>>> bool
>>>>>>> -source "drivers/char/agp/Kconfig"
>>>>>>> +if HAS_IOMEM
>>>>>>> + source "drivers/char/agp/Kconfig"
>>>>>>> -source "drivers/gpu/vga/Kconfig"
>>>>>>> + source "drivers/gpu/vga/Kconfig"
>>>>>>> -source "drivers/gpu/host1x/Kconfig"
>>>>>>> -source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>> + source "drivers/gpu/host1x/Kconfig"
>>>>>>> + source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>> -source "drivers/gpu/drm/Kconfig"
>>>>>>> + source "drivers/gpu/drm/Kconfig"
>>>>
>>>>
>>>> Hmmm, looks like that this makes it impossible to select VIRTIO_GPU - need
>>>> still more
>>>> work.
>>>>
>>> Sorry my fault. I had CONFIG_PCI disabled.
>>
>> That smells like the s390 HAS_IOMEM stuff needs more work -- I guess
>> that you want to enable a ccw virtio-gpu device, not a pci one, right?
>
> It is a ccw virtio-gpu. But s390 has no IOMEM without CONFIG_PCI, so you
> cannot
> select VIRTIO_GPU, which needs DRM, which need IOMEM.
So the 'source "drivers/gpu/drm/Kconfig"' should maybe rather reside
outside of the "if HAS_IOMEM" path? Or does it not compile anymore that way?
Thomas
Re: [PATCH v3 1/3] Kconfig : Remove HAS_IOMEM dependency for Graphics support
On 21.02.2018 12:09, Christian Borntraeger wrote:
>
>
> On 02/21/2018 11:32 AM, Cornelia Huck wrote:
>> On Wed, 21 Feb 2018 11:22:38 +0100
>> Christian Borntraeger wrote:
>>
>>> On 02/21/2018 11:05 AM, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 02/19/2018 05:38 PM, Farhan Ali wrote:
>>>>>
>>>>>
>>>>> On 02/19/2018 11:25 AM, Thomas Huth wrote:
>>>>>> On 19.02.2018 16:47, Farhan Ali wrote:
>>>>>>> The 'commit e25df1205f37 ("[S390] Kconfig: menus with depends on
>>>>>>> HAS_IOMEM.")'
>>>>>>> added the HAS_IOMEM dependecy for "Graphics support". This disabled the
>>>>>>> "Graphics support" menu for S390. But if we enable VT layer for S390,
>>>>>>> we would also need to enable the dummy console. So let's remove the
>>>>>>> HAS_IOMEM dependency.
>>>>>>>
>>>>>>> Move this dependency to sub menu items and console drivers that use
>>>>>>> io memory.
>>>>>>>
>>>>>>> Signed-off-by: Farhan Ali
>>>>>>> ---
>>>>>>> drivers/video/Kconfig | 21 +++--
>>>>>>> drivers/video/console/Kconfig | 4 ++--
>>>>>>> 2 files changed, 13 insertions(+), 12 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>>>>>> index 3c20af9..8f10915 100644
>>>>>>> --- a/drivers/video/Kconfig
>>>>>>> +++ b/drivers/video/Kconfig
>>>>>>> @@ -3,7 +3,6 @@
>>>>>>> #
>>>>>>> menu "Graphics support"
>>>>>>> - depends on HAS_IOMEM
>>>>>>> config HAVE_FB_ATMEL
>>>>>>> bool
>>>>>>> @@ -11,20 +10,22 @@ config HAVE_FB_ATMEL
>>>>>>> config SH_LCD_MIPI_DSI
>>>>>>> bool
>>>>>>> -source "drivers/char/agp/Kconfig"
>>>>>>> +if HAS_IOMEM
>>>>>>> + source "drivers/char/agp/Kconfig"
>>>>>>> -source "drivers/gpu/vga/Kconfig"
>>>>>>> + source "drivers/gpu/vga/Kconfig"
>>>>>>> -source "drivers/gpu/host1x/Kconfig"
>>>>>>> -source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>> + source "drivers/gpu/host1x/Kconfig"
>>>>>>> + source "drivers/gpu/ipu-v3/Kconfig"
>>>>>>> -source "drivers/gpu/drm/Kconfig"
>>>>>>> + source "drivers/gpu/drm/Kconfig"
>>>>
>>>>
>>>> Hmmm, looks like that this makes it impossible to select VIRTIO_GPU - need
>>>> still more
>>>> work.
>>>>
>>> Sorry my fault. I had CONFIG_PCI disabled.
>>
>> That smells like the s390 HAS_IOMEM stuff needs more work -- I guess
>> that you want to enable a ccw virtio-gpu device, not a pci one, right?
>
> It is a ccw virtio-gpu. But s390 has no IOMEM without CONFIG_PCI, so you
> cannot
> select VIRTIO_GPU, which needs DRM, which need IOMEM.
So the 'source "drivers/gpu/drm/Kconfig"' should maybe rather reside
outside of the "if HAS_IOMEM" path? Or does it not compile anymore that way?
Thomas
