[kvm-unit-tests Patch v1 0/2] x86: Test IOAPIC physical and logical destination mode

[kvm-unit-tests Patch v1 0/2] x86: Test IOAPIC physical and logical destination mode

[Nitesh Narayan Lal]

The first patch adds smp configuration to ioapic unittest and
the second one adds the support to test both physical and logical destination
modes under fixed delivery mode.

Nitesh Narayan Lal (2):
  x86: ioapic: Add the smp configuration to ioapic unittest
  x86: ioapic: Test physical and logical destination mode

 x86/ioapic.c      | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 x86/unittests.cfg |  1 +
 2 files changed, 66 insertions(+)

[kvm-unit-tests Patch v1 1/2] x86: ioapic: Add the smp configuration to ioapic unittest

[Nitesh Narayan Lal]

Adds -smp configuration for ioapic unittest. This is essentially
required to test IOAPIC Logical destination mode, where we send
an interrupt to more than one vcpus at the same time.

Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
---
 x86/unittests.cfg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 5ecb9bb..6ed0b86 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -45,6 +45,7 @@ timeout = 30
 
 [ioapic]
 file = ioapic.flat
+smp = 4
 extra_params = -cpu qemu64
 arch = x86_64
 
-- 
1.8.3.1

[kvm-unit-tests Patch v1 2/2] x86: ioapic: Test physical and logical destination mode

[Nitesh Narayan Lal]

This patch tests the physical destination mode by sending an
interrupt to one of the vcpus and logical destination mode by
sending an interrupt to more than one vcpus.

Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
---
 x86/ioapic.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/x86/ioapic.c b/x86/ioapic.c
index c32dabd..31aec03 100644
--- a/x86/ioapic.c
+++ b/x86/ioapic.c
@@ -405,12 +405,73 @@ static void test_ioapic_self_reconfigure(void)
 	report("Reconfigure self", g_isr_84 == 1 && e.remote_irr == 0);
 }
 
+static volatile int g_isr_85;
+
+static void ioapic_isr_85(isr_regs_t *regs)
+{
+	++g_isr_85;
+	set_irq_line(0x0e, 0);
+	eoi();
+}
+
+static void test_ioapic_physical_destination_mode(void)
+{
+	ioapic_redir_entry_t e = {
+		.vector = 0x85,
+		.delivery_mode = 0,
+		.dest_mode = 0,
+		.dest_id = 0x1,
+		.trig_mode = TRIGGER_LEVEL,
+	};
+	handle_irq(0x85, ioapic_isr_85);
+	ioapic_write_redir(0xe, e);
+	set_irq_line(0x0e, 1);
+	do {
+		pause();
+	} while(g_isr_85 != 1);
+	report("ioapic physical destination mode", g_isr_85 == 1);
+}
+
+static volatile int g_isr_86;
+
+static void ioapic_isr_86(isr_regs_t *regs)
+{
+	++g_isr_86;
+	set_irq_line(0x0e, 0);
+	eoi();
+}
+
+static void test_ioapic_logical_destination_mode(void)
+{
+	/* Number of vcpus which are configured/set in dest_id */
+	int nr_vcpus = 3;
+	ioapic_redir_entry_t e = {
+		.vector = 0x86,
+		.delivery_mode = 0,
+		.dest_mode = 1,
+		.dest_id = 0xd,
+		.trig_mode = TRIGGER_LEVEL,
+	};
+	handle_irq(0x86, ioapic_isr_86);
+	ioapic_write_redir(0xe, e);
+	set_irq_line(0x0e, 1);
+	do {
+		pause();
+	} while(g_isr_86 < nr_vcpus);
+	report("ioapic logical destination mode", g_isr_86 == nr_vcpus);
+}
+
+static void update_cr3(void *cr3)
+{
+	write_cr3((ulong)cr3);
+}
 
 int main(void)
 {
 	setup_vm();
 	smp_init();
 
+	on_cpus(update_cr3, (void *)read_cr3());
 	mask_pic_interrupts();
 
 	if (enable_x2apic())
@@ -448,7 +509,11 @@ int main(void)
 		test_ioapic_edge_tmr_smp(true);
 
 		test_ioapic_self_reconfigure();
+		test_ioapic_physical_destination_mode();
 	}
 
+	if (cpu_count() > 3)
+		test_ioapic_logical_destination_mode();
+
 	return report_summary();
 }
-- 
1.8.3.1

Re: [kvm-unit-tests Patch v1 2/2] x86: ioapic: Test physical and logical destination mode

[Thomas Huth]

On 06/11/2019 13.47, Nitesh Narayan Lal wrote:
> This patch tests the physical destination mode by sending an
> interrupt to one of the vcpus and logical destination mode by
> sending an interrupt to more than one vcpus.
> 
> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
> ---
>   x86/ioapic.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 65 insertions(+)

  Hi,

I think this patch likely broke the possibility to run the ioapic test 
under TCG (it was still working some weeks ago):

  https://gitlab.com/huth/kvm-unit-tests/-/jobs/363553211#L1815

Do you care about this test to be runnable under TCG, or shall I simply 
cook a patch to disable it in the gitlab CI?

  Thomas

Re: [kvm-unit-tests Patch v1 2/2] x86: ioapic: Test physical and logical destination mode

[Nitesh Narayan Lal]

On 11/28/19 1:38 AM, Thomas Huth wrote:
> On 06/11/2019 13.47, Nitesh Narayan Lal wrote:
>> This patch tests the physical destination mode by sending an
>> interrupt to one of the vcpus and logical destination mode by
>> sending an interrupt to more than one vcpus.
>>
>> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
>> ---
>>   x86/ioapic.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 65 insertions(+)
>
>  Hi,
>
> I think this patch likely broke the possibility to run the ioapic test under
> TCG (it was still working some weeks ago):
>
>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/363553211#L1815

Hi Thomas,

Not sure about the cause of it. I will take a look.

>
> Do you care about this test to be runnable under TCG, or shall I simply cook a
> patch to disable it in the gitlab CI?
>
>  Thomas
-- 
Thanks
Nitesh

Re: [kvm-unit-tests Patch v1 2/2] x86: ioapic: Test physical and logical destination mode

[Nitesh Narayan Lal]

On 11/28/19 1:38 AM, Thomas Huth wrote:
> On 06/11/2019 13.47, Nitesh Narayan Lal wrote:
>> This patch tests the physical destination mode by sending an
>> interrupt to one of the vcpus and logical destination mode by
>> sending an interrupt to more than one vcpus.
>>
>> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
>> ---
>>   x86/ioapic.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 65 insertions(+)
>
>  Hi,
>
> I think this patch likely broke the possibility to run the ioapic test under
> TCG (it was still working some weeks ago):
>
>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/363553211#L1815
>
> Do you care about this test to be runnable under TCG, or shall I simply cook a
> patch to disable it in the gitlab CI?


Hi Thomas,

I don't think there is a need for this test to be runnable under TCG. So we can
skip this for it.

>
>  Thomas
-- 
Thanks
Nitesh

Re: [kvm-unit-tests Patch v1 2/2] x86: ioapic: Test physical and logical destination mode

[Christophe de Dinechin]

Nitesh Narayan Lal writes:

> This patch tests the physical destination mode by sending an
> interrupt to one of the vcpus and logical destination mode by
> sending an interrupt to more than one vcpus.
>
> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
> ---
>  x86/ioapic.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/x86/ioapic.c b/x86/ioapic.c
> index c32dabd..31aec03 100644
> --- a/x86/ioapic.c
> +++ b/x86/ioapic.c
> @@ -405,12 +405,73 @@ static void test_ioapic_self_reconfigure(void)
>  	report("Reconfigure self", g_isr_84 == 1 && e.remote_irr == 0);
>  }
>
> +static volatile int g_isr_85;
> +
> +static void ioapic_isr_85(isr_regs_t *regs)
> +{
> +	++g_isr_85;
> +	set_irq_line(0x0e, 0);
> +	eoi();
> +}
> +
> +static void test_ioapic_physical_destination_mode(void)
> +{
> +	ioapic_redir_entry_t e = {
> +		.vector = 0x85,
> +		.delivery_mode = 0,
> +		.dest_mode = 0,
> +		.dest_id = 0x1,
> +		.trig_mode = TRIGGER_LEVEL,
> +	};
> +	handle_irq(0x85, ioapic_isr_85);
> +	ioapic_write_redir(0xe, e);
> +	set_irq_line(0x0e, 1);
> +	do {
> +		pause();
> +	} while(g_isr_85 != 1);

Does this loop (and the next one) end up running forever if the test
fails? Would it be worth adding some timeout to detect failure?

> +	report("ioapic physical destination mode", g_isr_85 == 1);
> +}
> +
> +static volatile int g_isr_86;
> +
> +static void ioapic_isr_86(isr_regs_t *regs)
> +{
> +	++g_isr_86;
> +	set_irq_line(0x0e, 0);
> +	eoi();
> +}
> +
> +static void test_ioapic_logical_destination_mode(void)
> +{
> +	/* Number of vcpus which are configured/set in dest_id */
> +	int nr_vcpus = 3;
> +	ioapic_redir_entry_t e = {
> +		.vector = 0x86,
> +		.delivery_mode = 0,
> +		.dest_mode = 1,
> +		.dest_id = 0xd,
> +		.trig_mode = TRIGGER_LEVEL,
> +	};
> +	handle_irq(0x86, ioapic_isr_86);
> +	ioapic_write_redir(0xe, e);
> +	set_irq_line(0x0e, 1);
> +	do {
> +		pause();
> +	} while(g_isr_86 < nr_vcpus);
> +	report("ioapic logical destination mode", g_isr_86 == nr_vcpus);
> +}
> +
> +static void update_cr3(void *cr3)
> +{
> +	write_cr3((ulong)cr3);
> +}
>
>  int main(void)
>  {
>  	setup_vm();
>  	smp_init();
>
> +	on_cpus(update_cr3, (void *)read_cr3());
>  	mask_pic_interrupts();
>
>  	if (enable_x2apic())
> @@ -448,7 +509,11 @@ int main(void)
>  		test_ioapic_edge_tmr_smp(true);
>
>  		test_ioapic_self_reconfigure();
> +		test_ioapic_physical_destination_mode();
>  	}
>
> +	if (cpu_count() > 3)
> +		test_ioapic_logical_destination_mode();
> +
>  	return report_summary();
>  }


--
Cheers,
Christophe de Dinechin (IRC c3d)

Re: [kvm-unit-tests Patch v1 2/2] x86: ioapic: Test physical and logical destination mode

[Nitesh Narayan Lal]

On 11/28/19 5:25 AM, Christophe de Dinechin wrote:
> Nitesh Narayan Lal writes:
>
>> This patch tests the physical destination mode by sending an
>> interrupt to one of the vcpus and logical destination mode by
>> sending an interrupt to more than one vcpus.
>>
>> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com>
>> ---
>>  x86/ioapic.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 65 insertions(+)
>>
>> diff --git a/x86/ioapic.c b/x86/ioapic.c
>> index c32dabd..31aec03 100644
>> --- a/x86/ioapic.c
>> +++ b/x86/ioapic.c
>> @@ -405,12 +405,73 @@ static void test_ioapic_self_reconfigure(void)
>>  	report("Reconfigure self", g_isr_84 == 1 && e.remote_irr == 0);
>>  }
>>
>> +static volatile int g_isr_85;
>> +
>> +static void ioapic_isr_85(isr_regs_t *regs)
>> +{
>> +	++g_isr_85;
>> +	set_irq_line(0x0e, 0);
>> +	eoi();
>> +}
>> +
>> +static void test_ioapic_physical_destination_mode(void)
>> +{
>> +	ioapic_redir_entry_t e = {
>> +		.vector = 0x85,
>> +		.delivery_mode = 0,
>> +		.dest_mode = 0,
>> +		.dest_id = 0x1,
>> +		.trig_mode = TRIGGER_LEVEL,
>> +	};
>> +	handle_irq(0x85, ioapic_isr_85);
>> +	ioapic_write_redir(0xe, e);
>> +	set_irq_line(0x0e, 1);
>> +	do {
>> +		pause();
>> +	} while(g_isr_85 != 1);
> Does this loop (and the next one) end up running forever if the test
> fails? Would it be worth adding some timeout to detect failure?

AFAIK there is already a timeout in place. i.e., if we don't receive an
interrupt then eventually the timeout will occur and the test will terminate.

>
>> +	report("ioapic physical destination mode", g_isr_85 == 1);
>> +}
>> +
>> +static volatile int g_isr_86;
>> +
>> +static void ioapic_isr_86(isr_regs_t *regs)
>> +{
>> +	++g_isr_86;
>> +	set_irq_line(0x0e, 0);
>> +	eoi();
>> +}
>> +
>> +static void test_ioapic_logical_destination_mode(void)
>> +{
>> +	/* Number of vcpus which are configured/set in dest_id */
>> +	int nr_vcpus = 3;
>> +	ioapic_redir_entry_t e = {
>> +		.vector = 0x86,
>> +		.delivery_mode = 0,
>> +		.dest_mode = 1,
>> +		.dest_id = 0xd,
>> +		.trig_mode = TRIGGER_LEVEL,
>> +	};
>> +	handle_irq(0x86, ioapic_isr_86);
>> +	ioapic_write_redir(0xe, e);
>> +	set_irq_line(0x0e, 1);
>> +	do {
>> +		pause();
>> +	} while(g_isr_86 < nr_vcpus);
>> +	report("ioapic logical destination mode", g_isr_86 == nr_vcpus);
>> +}
>> +
>> +static void update_cr3(void *cr3)
>> +{
>> +	write_cr3((ulong)cr3);
>> +}
>>
>>  int main(void)
>>  {
>>  	setup_vm();
>>  	smp_init();
>>
>> +	on_cpus(update_cr3, (void *)read_cr3());
>>  	mask_pic_interrupts();
>>
>>  	if (enable_x2apic())
>> @@ -448,7 +509,11 @@ int main(void)
>>  		test_ioapic_edge_tmr_smp(true);
>>
>>  		test_ioapic_self_reconfigure();
>> +		test_ioapic_physical_destination_mode();
>>  	}
>>
>> +	if (cpu_count() > 3)
>> +		test_ioapic_logical_destination_mode();
>> +
>>  	return report_summary();
>>  }
>
> --
> Cheers,
> Christophe de Dinechin (IRC c3d)
>
-- 
Thanks
Nitesh

Re: [kvm-unit-tests Patch v1 0/2] x86: Test IOAPIC physical and logical destination mode

[Nitesh Narayan Lal]

On 11/6/19 7:47 AM, Nitesh Narayan Lal wrote:
> The first patch adds smp configuration to ioapic unittest and
> the second one adds the support to test both physical and logical destination
> modes under fixed delivery mode.
>
> Nitesh Narayan Lal (2):
>   x86: ioapic: Add the smp configuration to ioapic unittest
>   x86: ioapic: Test physical and logical destination mode
>
>  x86/ioapic.c      | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  x86/unittests.cfg |  1 +
>  2 files changed, 66 insertions(+)

I just wanted to check if there are any more suggestions or comments which I
need to address before this can be merged?

-- 
Thanks
Nitesh

Re: [kvm-unit-tests Patch v1 0/2] x86: Test IOAPIC physical and logical destination mode

[Marcelo Tosatti]

On Wed, Nov 06, 2019 at 07:47:07AM -0500, Nitesh Narayan Lal wrote:
> The first patch adds smp configuration to ioapic unittest and
> the second one adds the support to test both physical and logical destination
> modes under fixed delivery mode.
> 
> Nitesh Narayan Lal (2):
>   x86: ioapic: Add the smp configuration to ioapic unittest
>   x86: ioapic: Test physical and logical destination mode
> 
>  x86/ioapic.c      | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  x86/unittests.cfg |  1 +
>  2 files changed, 66 insertions(+)

Looks good to me.

Re: [kvm-unit-tests Patch v1 0/2] x86: Test IOAPIC physical and logical destination mode

[Paolo Bonzini]

On 06/11/19 13:47, Nitesh Narayan Lal wrote:
> The first patch adds smp configuration to ioapic unittest and
> the second one adds the support to test both physical and logical destination
> modes under fixed delivery mode.
> 
> Nitesh Narayan Lal (2):
>   x86: ioapic: Add the smp configuration to ioapic unittest
>   x86: ioapic: Test physical and logical destination mode
> 
>  x86/ioapic.c      | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  x86/unittests.cfg |  1 +
>  2 files changed, 66 insertions(+)
> 

Applied, thanks.

Paolo