[PATCH V2] arm64: KVM: Fix HCR_EL2 and VTCR_EL2 configuration bits

[Pranavkumar Sawargaonkar]

Hi Christopher,

On 13 May 2013 22:46, Christopher Covington <cov at codeaurora.org> wrote:
> On 05/13/2013 12:02 PM, Marc Zyngier wrote:
>> On Mon, 13 May 2013 11:23:05 -0400, Christopher Covington
>> <cov at codeaurora.org> wrote:
>>> Hi Pranavkumar,
>>>
>>> On 05/10/2013 03:23 AM, Pranavkumar Sawargaonkar wrote:
>>>> This patch does following fixes:
>>>>
>>>> 1. Make HCR_* flags as unsigned long long constants
>>>> Reason : By default, compiler assumes numeric constants as
>>>> signed hence sign extending it when assigned to unsigned variable
>>>> such as hcr_el2 (in VCPU context). This accidently sets HCR_ID and
>>>> HCR_CD making entire guest memory non-cacheable. On real HW, this
>>>> breaks Stage2 ttbl walks and also breaks VirtIO.
>>>>
>>>> 2. VTCR_EL2_ORGN0_WBWA and VTCR_EL2_IRGN0_WBWA macros.
>>>>
>>>> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar at linaro.org>
>>>> Signed-off-by: Anup Patel <anup.patel at linaro.org>
>>>> ---
>>>>  arch/arm64/include/asm/kvm_arm.h |   73
>>>>  +++++++++++++++++++-------------------
>>>>  1 file changed, 37 insertions(+), 36 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/include/asm/kvm_arm.h
>>>> b/arch/arm64/include/asm/kvm_arm.h
>>>> index 8ced0ca..14ead69 100644
>>>> --- a/arch/arm64/include/asm/kvm_arm.h
>>>> +++ b/arch/arm64/include/asm/kvm_arm.h
>>>> @@ -18,44 +18,45 @@
>>>>  #ifndef __ARM64_KVM_ARM_H__
>>>>  #define __ARM64_KVM_ARM_H__
>>>>
>>>> +#include <linux/const.h>
>>>>  #include <asm/types.h>
>>>>
>>>>  /* Hyp Configuration Register (HCR) bits */
>>>> -#define HCR_ID             (1 << 33)
>>>> -#define HCR_CD             (1 << 32)
>>>> +#define HCR_ID             (UL(0x1) << 33)
>>>> +#define HCR_CD             (UL(0x1) << 32)
>>>
>>> Consider using the BIT() macro from linux/bitops.h to improve
>> readability.
>>
>> This would probably cause problems when using these macros from assembly
>> code.
>
> It does indeed cause problems. What a shame. How about a quick local
> definition, to perhaps be eventually replaced by a more accessible shared
> definition?

I have used UL to be synced with current arm64 code style (it has been
used in other files also).
Using BIT is good idea (not sure about local definition i.e. how good
it is when we have one inside bitops.h)
but if we want to use it is, then better idea is to use it everywhere.
>
> Christopher

Thanks,
Pranav

>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by the Linux Foundation.