On 14/07/16 11:00, Andre Przywara wrote:
> Hi,
>
> On 14/07/16 10:46, Marc Zyngier wrote:
>> On 13/07/16 02:59, Andre Przywara wrote:
>>> The (system-wide) LPI configuration table is held in a table in
>>> (guest) memory. To achieve reasonable performance, we cache this data
>>> in our struct vgic_irq. If the guest updates the configuration data
>>> (which consists of the enable bit and the priority value), it issues
>>> an INV or INVALL command to allow us to update our information.
>>> Provide functions that update that information for one LPI or all LPIs
>>> mapped to a specific collection.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>>> ---
>>> virt/kvm/arm/vgic/vgic-its.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 39 insertions(+)
>>>
>>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>>> index f400ef1..60108f8 100644
>>> --- a/virt/kvm/arm/vgic/vgic-its.c
>>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>>> @@ -64,6 +64,45 @@ struct its_itte {
>>>
>>> #define CBASER_ADDRESS(x) ((x) & GENMASK_ULL(51, 12))
>>> #define PENDBASER_ADDRESS(x) ((x) & GENMASK_ULL(51, 16))
>>> +#define PROPBASER_ADDRESS(x) ((x) & GENMASK_ULL(51, 12))
>>
>> Again, these masks are wrong as we limit the ITS to 48 bits PA.
>
> Those masks match the architecture description of the register. We limit
> to 48 bits of PA when the guest writes to those registers. I'd like to
> keep this limitation in one place only.
Which makes it hard to understand for everyone. If you want something
truly generic, assign proper names to these defines:
#define CBASER_ARCHITECTURAL_PA_BITS 52
#define CBASER_IMPLEMENTATION_PA_BITS 48
and repaint all your defines to use macros along those lines. If not,
just change everything to 48 and let's be done with it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
