x86/amd: Initial support for Fam19h processors
diff mbox series

Message ID 20200430095947.31958-1-andrew.cooper3@citrix.com
State New
Headers show
Series
  • x86/amd: Initial support for Fam19h processors
Related show

Commit Message

Andrew Cooper April 30, 2020, 9:59 a.m. UTC
Fam19h is very similar to Fam17h in these regards.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/acpi/cpu_idle.c     | 1 +
 xen/arch/x86/cpu/microcode/amd.c | 1 +
 xen/arch/x86/cpu/vpmu_amd.c      | 1 +
 xen/arch/x86/nmi.c               | 2 +-
 xen/arch/x86/traps.c             | 2 +-
 5 files changed, 5 insertions(+), 2 deletions(-)

Comments

Roger Pau Monné April 30, 2020, 10:35 a.m. UTC | #1
On Thu, Apr 30, 2020 at 10:59:47AM +0100, Andrew Cooper wrote:
> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
> index c3f92ed231..014524486f 100644
> --- a/xen/arch/x86/nmi.c
> +++ b/xen/arch/x86/nmi.c
> @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void)
>      case X86_VENDOR_AMD:
>          switch (boot_cpu_data.x86) {
>          case 6:
> -        case 0xf ... 0x17:
> +        case 0xf ... 0x19:
>              setup_k7_watchdog();
>              break;
>          default:
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index 0bcf554e93..33e5d21ece 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void)
>          switch ( boot_cpu_data.x86 )
>          {
>          case 6:
> -        case 0xf ... 0x17:
> +        case 0xf ... 0x19:
>              return MSR_IA32_LASTINTFROMIP;

You seem to also add support for Fam18h here and in the chunk above,
is this intentional?

Thanks, Roger.
Andrew Cooper April 30, 2020, 10:38 a.m. UTC | #2
On 30/04/2020 11:35, Roger Pau Monné wrote:
> On Thu, Apr 30, 2020 at 10:59:47AM +0100, Andrew Cooper wrote:
>> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
>> index c3f92ed231..014524486f 100644
>> --- a/xen/arch/x86/nmi.c
>> +++ b/xen/arch/x86/nmi.c
>> @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void)
>>      case X86_VENDOR_AMD:
>>          switch (boot_cpu_data.x86) {
>>          case 6:
>> -        case 0xf ... 0x17:
>> +        case 0xf ... 0x19:
>>              setup_k7_watchdog();
>>              break;
>>          default:
>> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
>> index 0bcf554e93..33e5d21ece 100644
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void)
>>          switch ( boot_cpu_data.x86 )
>>          {
>>          case 6:
>> -        case 0xf ... 0x17:
>> +        case 0xf ... 0x19:
>>              return MSR_IA32_LASTINTFROMIP;
> You seem to also add support for Fam18h here and in the chunk above,
> is this intentional?

Yes.  Honestly, these details have never changed since the K7.  I'm
tempted to drop the family logic entirely.

~Andrew
Roger Pau Monné April 30, 2020, 10:41 a.m. UTC | #3
On Thu, Apr 30, 2020 at 11:38:14AM +0100, Andrew Cooper wrote:
> On 30/04/2020 11:35, Roger Pau Monné wrote:
> > On Thu, Apr 30, 2020 at 10:59:47AM +0100, Andrew Cooper wrote:
> >> diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
> >> index c3f92ed231..014524486f 100644
> >> --- a/xen/arch/x86/nmi.c
> >> +++ b/xen/arch/x86/nmi.c
> >> @@ -398,7 +398,7 @@ void setup_apic_nmi_watchdog(void)
> >>      case X86_VENDOR_AMD:
> >>          switch (boot_cpu_data.x86) {
> >>          case 6:
> >> -        case 0xf ... 0x17:
> >> +        case 0xf ... 0x19:
> >>              setup_k7_watchdog();
> >>              break;
> >>          default:
> >> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> >> index 0bcf554e93..33e5d21ece 100644
> >> --- a/xen/arch/x86/traps.c
> >> +++ b/xen/arch/x86/traps.c
> >> @@ -1939,7 +1939,7 @@ static unsigned int calc_ler_msr(void)
> >>          switch ( boot_cpu_data.x86 )
> >>          {
> >>          case 6:
> >> -        case 0xf ... 0x17:
> >> +        case 0xf ... 0x19:
> >>              return MSR_IA32_LASTINTFROMIP;
> > You seem to also add support for Fam18h here and in the chunk above,
> > is this intentional?
> 
> Yes.  Honestly, these details have never changed since the K7.  I'm
> tempted to drop the family logic entirely.

Ack, just wanted to be sure the changes where intentional:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Re dropping the logic - as you wish.

Thanks, Roger.
Jan Beulich April 30, 2020, 11:09 a.m. UTC | #4
On 30.04.2020 11:59, Andrew Cooper wrote:
> Fam19h is very similar to Fam17h in these regards.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Nevertheless a question:

> --- a/xen/arch/x86/cpu/microcode/amd.c
> +++ b/xen/arch/x86/cpu/microcode/amd.c
> @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size)
>          max_size = F16H_MPB_MAX_SIZE;
>          break;
>      case 0x17:
> +    case 0x19:
>          max_size = F17H_MPB_MAX_SIZE;
>          break;

Didn't you indicate to me the other day that the upper bound would
grow?

Jan
Andrew Cooper April 30, 2020, 3:50 p.m. UTC | #5
On 30/04/2020 12:09, Jan Beulich wrote:
> On 30.04.2020 11:59, Andrew Cooper wrote:
>> Fam19h is very similar to Fam17h in these regards.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

>
> Nevertheless a question:
>
>> --- a/xen/arch/x86/cpu/microcode/amd.c
>> +++ b/xen/arch/x86/cpu/microcode/amd.c
>> @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size)
>>          max_size = F16H_MPB_MAX_SIZE;
>>          break;
>>      case 0x17:
>> +    case 0x19:
>>          max_size = F17H_MPB_MAX_SIZE;
>>          break;
> Didn't you indicate to me the other day that the upper bound would
> grow?

That was a very non-specific patch to Linux.  I've asked around, and the
answer seems to be 4800.

Are you happy for your review to stand with adding a new
F19H_MPB_MAX_SIZE define to this effect?

~Andrew
Jan Beulich May 4, 2020, 8:53 a.m. UTC | #6
On 30.04.2020 17:50, Andrew Cooper wrote:
> On 30/04/2020 12:09, Jan Beulich wrote:
>> On 30.04.2020 11:59, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/cpu/microcode/amd.c
>>> +++ b/xen/arch/x86/cpu/microcode/amd.c
>>> @@ -125,6 +125,7 @@ static bool_t verify_patch_size(uint32_t patch_size)
>>>          max_size = F16H_MPB_MAX_SIZE;
>>>          break;
>>>      case 0x17:
>>> +    case 0x19:
>>>          max_size = F17H_MPB_MAX_SIZE;
>>>          break;
>> Didn't you indicate to me the other day that the upper bound would
>> grow?
> 
> That was a very non-specific patch to Linux.  I've asked around, and the
> answer seems to be 4800.
> 
> Are you happy for your review to stand with adding a new
> F19H_MPB_MAX_SIZE define to this effect?

Yes.

Jan

Patch
diff mbox series

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index e00f2a82de..b83446e77d 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -1356,6 +1356,7 @@  static void amd_cpuidle_init(struct acpi_processor_power *power)
 
     switch ( c->x86 )
     {
+    case 0x19:
     case 0x18:
         if ( boot_cpu_data.x86_vendor != X86_VENDOR_HYGON )
         {
diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 13bf9f4dee..89b656bd2b 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -125,6 +125,7 @@  static bool_t verify_patch_size(uint32_t patch_size)
         max_size = F16H_MPB_MAX_SIZE;
         break;
     case 0x17:
+    case 0x19:
         max_size = F17H_MPB_MAX_SIZE;
         break;
     default:
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index 3c6799b42c..eba47cd2a0 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -576,6 +576,7 @@  int __init amd_vpmu_init(void)
     {
     case 0x15:
     case 0x17:
+    case 0x19:
         num_counters = F15H_NUM_COUNTERS;
         counters = AMD_F15H_COUNTERS;
         ctrls = AMD_F15H_CTRLS;
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index c3f92ed231..014524486f 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -398,7 +398,7 @@  void setup_apic_nmi_watchdog(void)
     case X86_VENDOR_AMD:
         switch (boot_cpu_data.x86) {
         case 6:
-        case 0xf ... 0x17:
+        case 0xf ... 0x19:
             setup_k7_watchdog();
             break;
         default:
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 0bcf554e93..33e5d21ece 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1939,7 +1939,7 @@  static unsigned int calc_ler_msr(void)
         switch ( boot_cpu_data.x86 )
         {
         case 6:
-        case 0xf ... 0x17:
+        case 0xf ... 0x19:
             return MSR_IA32_LASTINTFROMIP;
         }
         break;