Mailing List Archive

[PATCH] x86/Intel: avoid UB with NMI watchdog on family 15 CPUs
Found by looking for patterns similar to the one Julien did spot in
pci_vtd_quirks().

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -132,7 +132,7 @@ int nmi_active;
#define P4_ESCR_EVENT_SELECT(N) ((N)<<25)
#define P4_CCCR_OVF_PMI0 (1<<26)
#define P4_CCCR_OVF_PMI1 (1<<27)
-#define P4_CCCR_OVF (1<<31)
+#define P4_CCCR_OVF (1u << 31)
#define P4_CCCR_THRESHOLD(N) ((N)<<20)
#define P4_CCCR_COMPLEMENT (1<<19)
#define P4_CCCR_COMPARE (1<<18)
Re: [PATCH] x86/Intel: avoid UB with NMI watchdog on family 15 CPUs [ In reply to ]
On 19/11/2020 15:57, Jan Beulich wrote:
> Found by looking for patterns similar to the one Julien did spot in
> pci_vtd_quirks().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Subject is wonky.  Is it P4 (Intel), or Fam15 (AMD) ?  I'd be tempted to
have the prefix as x86/nmi: either way.

With that suitably adjusted, Acked-by: Andrew Cooper
<andrew.cooper3@citrix.com>
Re: [PATCH] x86/Intel: avoid UB with NMI watchdog on family 15 CPUs [ In reply to ]
On 19.11.2020 17:10, Andrew Cooper wrote:
> On 19/11/2020 15:57, Jan Beulich wrote:
>> Found by looking for patterns similar to the one Julien did spot in
>> pci_vtd_quirks().
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Subject is wonky.  Is it P4 (Intel), or Fam15 (AMD) ?  I'd be tempted to
> have the prefix as x86/nmi: either way.

With this code:

case X86_VENDOR_INTEL:
switch (boot_cpu_data.x86) {
case 6:
setup_p6_watchdog((boot_cpu_data.x86_model < 14)
? P6_EVENT_CPU_CLOCKS_NOT_HALTED
: CORE_EVENT_CPU_CLOCKS_NOT_HALTED);
break;
case 15:
if (!setup_p4_watchdog())

I think qualifying it like I did is quite reasonable. Hence ...

> With that suitably adjusted, Acked-by: Andrew Cooper
> <andrew.cooper3@citrix.com>

... I'd prefer to keep it as is - please clarify.

Jan
Re: [PATCH] x86/Intel: avoid UB with NMI watchdog on family 15 CPUs [ In reply to ]
On 19/11/2020 16:37, Jan Beulich wrote:
> On 19.11.2020 17:10, Andrew Cooper wrote:
>> On 19/11/2020 15:57, Jan Beulich wrote:
>>> Found by looking for patterns similar to the one Julien did spot in
>>> pci_vtd_quirks().
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Subject is wonky.  Is it P4 (Intel), or Fam15 (AMD) ?  I'd be tempted to
>> have the prefix as x86/nmi: either way.
> With this code:
>
> case X86_VENDOR_INTEL:
> switch (boot_cpu_data.x86) {
> case 6:
> setup_p6_watchdog((boot_cpu_data.x86_model < 14)
> ? P6_EVENT_CPU_CLOCKS_NOT_HALTED
> : CORE_EVENT_CPU_CLOCKS_NOT_HALTED);
> break;
> case 15:
> if (!setup_p4_watchdog())
>
> I think qualifying it like I did is quite reasonable. Hence ...
>
>> With that suitably adjusted, Acked-by: Andrew Cooper
>> <andrew.cooper3@citrix.com>
> ... I'd prefer to keep it as is - please clarify.

Oh - original Xeon's.  I'd honestly forgotten that quirk of history.

I'd recommend "x86/nmi: Avoid UB in for P4-era watchdogs" to avoid the
ambiguity altogether.

~Andrew
Re: [PATCH] x86/Intel: avoid UB with NMI watchdog on family 15 CPUs [ In reply to ]
On 19/11/2020 17:10, Andrew Cooper wrote:
> On 19/11/2020 16:37, Jan Beulich wrote:
>> On 19.11.2020 17:10, Andrew Cooper wrote:
>>> On 19/11/2020 15:57, Jan Beulich wrote:
>>>> Found by looking for patterns similar to the one Julien did spot in
>>>> pci_vtd_quirks().
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Subject is wonky.  Is it P4 (Intel), or Fam15 (AMD) ?  I'd be tempted to
>>> have the prefix as x86/nmi: either way.
>> With this code:
>>
>> case X86_VENDOR_INTEL:
>> switch (boot_cpu_data.x86) {
>> case 6:
>> setup_p6_watchdog((boot_cpu_data.x86_model < 14)
>> ? P6_EVENT_CPU_CLOCKS_NOT_HALTED
>> : CORE_EVENT_CPU_CLOCKS_NOT_HALTED);
>> break;
>> case 15:
>> if (!setup_p4_watchdog())
>>
>> I think qualifying it like I did is quite reasonable. Hence ...
>>
>>> With that suitably adjusted, Acked-by: Andrew Cooper
>>> <andrew.cooper3@citrix.com>
>> ... I'd prefer to keep it as is - please clarify.
> Oh - original Xeon's.  I'd honestly forgotten that quirk of history.
>
> I'd recommend "x86/nmi: Avoid UB in for P4-era watchdogs" to avoid the
> ambiguity altogether.

And if I could actually english, that would read "Avoid UB for P4-".

~Andrew