On 14/07/16 11:00, Andre Przywara wrote:

Hi,

On 14/07/16 10:46, Marc Zyngier wrote:

quoted

On 13/07/16 02:59, Andre Przywara wrote:

quoted

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@arm.com>
---
 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...