[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 01/12] xen/arm: enable SVE extension for Xen


  • To: Julien Grall <julien@xxxxxxx>
  • From: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • Date: Mon, 22 May 2023 09:35:34 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=bAnErE0iPmulMM7K+O4tVeGFFSPj/8XYBKtQ54e1JQU=; b=Er+3HxUf2+oy7OK0tcISmp9UAKXe8GIr13mJMV3ni+ol7nknO7exEkpn/m3db9ViUZy1yAGmdMzlHTx63s68iAGJ9/fD9LAzctzaH9esi/fF8Bq35sCRIxx/6dTWfm31OhpVnfLsb+pkXcJJrzjst+6EBB60uxcZpeZb5957t7lEVeKZ1AfGVbT7fvkSpizYJKajzl7Bfs144yf7KMCg+jz78ZKwhT65g8uk9aw0XC2S0Yu2c/Y1c0HgHb7UFaVhA6gyKzkgDeuZuddiRDRy6/PDpkVDiM39cDp2Woa5syhPVZyRJ+CSA2MWp2PkDp4ZI51wuT7TGKUtV9gO8bcnOA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=g79McFVoyUbTqYtvJIXDuYnnlIvtKd5tuBT2R6RwE8Qhe/8cGIIKCtiVi9w0ohcWSMQhsjTeutc9kMh65yUA0Hzp/c0qTWl8/P5g1Wsm9RZr2dLXhEpA6UgTs1g8Q9vIN++8OpC7PHnQcUAOS4TyBB874Gdgte1kR/pH0ME+C6XTUMKdivBVMuk53bpHT9DM+hpfGF1ske0zsn0al9yZUU420X74d/rrNSzrFv9YlkwKefkh7OVgiCK8arAn010N1nJtq8O6fgR8lb43u3TjD7arvu7I/AFXxGNAtUmxpz+vqfRLcE1XTYZy0h34vva2cOBTyIW5I0pkYSy3fNh6Hw==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 22 May 2023 09:36:10 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHZdnKBJmFHv6Wq8kSjvXctD+zlI69f6zCAgAHjioCAAAXEgIAEQtyAgAAOngCAAA0iAIAAAXeA
  • Thread-topic: [PATCH v6 01/12] xen/arm: enable SVE extension for Xen


> On 22 May 2023, at 10:30, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi,
> 
> On 22/05/2023 09:43, Luca Fancellu wrote:
>>> On 22 May 2023, at 08:50, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> 
>>> On 19.05.2023 16:46, Julien Grall wrote:
>>>> On 19/05/2023 15:26, Luca Fancellu wrote:
>>>>>> On 18 May 2023, at 10:35, Julien Grall <julien@xxxxxxx> wrote:
>>>>>>> +/*
>>>>>>> + * Arm SVE feature code
>>>>>>> + *
>>>>>>> + * Copyright (C) 2022 ARM Ltd.
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <xen/types.h>
>>>>>>> +#include <asm/arm64/sve.h>
>>>>>>> +#include <asm/arm64/sysregs.h>
>>>>>>> +#include <asm/processor.h>
>>>>>>> +#include <asm/system.h>
>>>>>>> +
>>>>>>> +extern unsigned int sve_get_hw_vl(void);
>>>>>>> +
>>>>>>> +register_t compute_max_zcr(void)
>>>>>>> +{
>>>>>>> +    register_t cptr_bits = get_default_cptr_flags();
>>>>>>> +    register_t zcr = vl_to_zcr(SVE_VL_MAX_BITS);
>>>>>>> +    unsigned int hw_vl;
>>>>>>> +
>>>>>>> +    /* Remove trap for SVE resources */
>>>>>>> +    WRITE_SYSREG(cptr_bits & ~HCPTR_CP(8), CPTR_EL2);
>>>>>>> +    isb();
>>>>>>> +
>>>>>>> +    /*
>>>>>>> +     * Set the maximum SVE vector length, doing that we will know the 
>>>>>>> VL
>>>>>>> +     * supported by the platform, calling sve_get_hw_vl()
>>>>>>> +     */
>>>>>>> +    WRITE_SYSREG(zcr, ZCR_EL2);
>>>>>> 
>>>>>> From my reading of the Arm (D19-6331, ARM DDI 0487J.a), a direct write 
>>>>>> to a system register would need to be followed by an context 
>>>>>> synchronization event (e.g. isb()) before the software can rely on the 
>>>>>> value.
>>>>>> 
>>>>>> In this situation, AFAICT, the instruciton in sve_get_hw_vl() will use 
>>>>>> the content of ZCR_EL2. So don't we need an ISB() here?
>>>>> 
>>>>> From what I’ve read in the manual for ZCR_ELx:
>>>>> 
>>>>> An indirect read of ZCR_EL2.LEN appears to occur in program order 
>>>>> relative to a direct write of
>>>>> the same register, without the need for explicit synchronization
>>>>> 
>>>>> I’ve interpreted it as “there is no need to sync before write” and I’ve 
>>>>> looked into Linux and it does not
>>>>> Appear any synchronisation mechanism after a write to that register, but 
>>>>> if I am wrong I can for sure
>>>>> add an isb if you prefer.
>>>> 
>>>> Ah, I was reading the generic section about synchronization and didn't
>>>> realize there was a paragraph in the ZCR_EL2 section as well.
>>>> 
>>>> Reading the new section, I agree with your understanding. The isb() is
>>>> not necessary.
>>> 
>>> And RDVL counts as an "indirect read"? I'm pretty sure "normal" SVE insn
>>> use is falling in that category, but RDVL might also be viewed as more
>>> similar to MRS in this regard? While the construct CurrentVL is used in
>>> either case, I'm still not sure this goes without saying.
>> Hi Jan,
>> Looking into the Linux code, in function vec_probe_vqs(...) in 
>> arch/arm64/kernel/fpsimd.c,
>> ZCR_EL1 is written, without synchronisation, and afterwards RDVL is used.
> 
> You are making the assumption that the Linux code is correct. It is mostly 
> likely the case, but in general it is best to justify barriers based on the 
> Arm Arm because it is authoritative.
> 
> In this case, the Arm Arm is pretty clear on the difference between indirect 
> read and direct read (see D19-63333 ARM DDI 0487J.A). The latter only refers 
> to use of the instruction of MRS. RDVL is its own instruction and therefore 
> this is an indirect read.

Yes you are right

> 
> Cheers,
> 
> -- 
> Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.