Re: [PATCH] target/ppc/spapr: Update H_GET_CPU_CHARACTERISTICS bits

[Nicholas Piggin <npiggin@gmail.com>]

Excerpts from David Gibson's message of May 4, 2021 10:41 am:
> On Mon, May 03, 2021 at 10:58:33PM +1000, Nicholas Piggin wrote:
>> There are several new bits added to the hcall which reflect new issues
>> found and new hardware mitigations.
>> 
>> This adds the link stack flush behaviour, link stack flush accelerated
>> instruction capability, and several L1D flush type behaviours (which are
>> now being specified as negative in order to simplify patched kernel
>> compatibility with older firmware).
> 
> So, to clarify here, the bits your adding aren't advertising any new
> behaviour on qemu/KVM's part, they're just new ways of advertising the
> same behaviour?

I... think so. "Behaviour" is in context of the hcall that advertises
how the processor behaves (or what the guest must do for security).

The new NO_ bits added are for processors that don't require a particular
flush. The FLUSH_LINK_STACK was basically always required but I think
Linux just keyed off the count cache flush and did both at once.

The new LINK_FLUSH_ASSIST is a new processor feature the guest will use
to implement link stack flushing, so maybe that does need a cap?

> 
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  hw/ppc/spapr_hcall.c   | 5 +++++
>>  include/hw/ppc/spapr.h | 6 ++++++
>>  2 files changed, 11 insertions(+)
>> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 7275d0bba1..f656620232 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1878,6 +1878,9 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>>          behaviour |= H_CPU_BEHAV_L1D_FLUSH_PR;
>>          break;
>>      case SPAPR_CAP_FIXED:
>> +        behaviour |= H_CPU_BEHAV_NO_L1D_FLUSH_ENTRY;
>> +        behaviour |= H_CPU_BEHAV_NO_L1D_FLUSH_UACCESS;
>> +        behaviour |= H_CPU_BEHAV_NO_STF_BARRIER;
>>          break;
>>      default: /* broken */
>>          assert(safe_cache == SPAPR_CAP_BROKEN);
>> @@ -1909,9 +1912,11 @@ static target_ulong h_get_cpu_characteristics(PowerPCCPU *cpu,
>>          break;
>>      case SPAPR_CAP_WORKAROUND:
>>          behaviour |= H_CPU_BEHAV_FLUSH_COUNT_CACHE;
>> +        behaviour |= H_CPU_BEHAV_FLUSH_LINK_STACK;
>>          if (count_cache_flush_assist) {
>>              characteristics |= H_CPU_CHAR_BCCTR_FLUSH_ASSIST;
>>          }
>> +        /* Should have a way to enable BCCTR_LINK_FLUSH_ASSIST */
> 
> Do we need a new spapr capability for this link flush thing?

It is independent of the FLUSH_COUNT_CACHE capability, so it seems like
it should I think? Should that be added as a following patch?

Thanks,
Nick