refactor: use bitfields for interrupt_descriptor struct
To remove the use of hardcoded values when setting the
attributes of the type_config_sec_state field in the
interrupt_descriptor, use bitfields so each field can
be set individually. This reduces the need for get and
set functions so remove them and just access the fields
in the struct directly.
Signed-off-by: Daniel Boulby <daniel.boulby@arm.com>
Change-Id: I66d2f292e4f64f654516649094f158f50625375f
diff --git a/inc/hf/ffa_partition_manifest.h b/inc/hf/ffa_partition_manifest.h
index c4f638c..e38f29c 100644
--- a/inc/hf/ffa_partition_manifest.h
+++ b/inc/hf/ffa_partition_manifest.h
@@ -93,6 +93,21 @@
uint32_t dma_access_permissions;
};
+/**
+ * Interrupts attibutes encoding in the manifest:
+ * Field Bit(s)
+ * ---------------------------
+ * Priority 7:0
+ * Security_State 8
+ * Config(Edge/Level) 9
+ * Type(SPI/PPI/SGI) 11:10
+ * Reserved 31:12
+ */
+#define INT_INFO_ATTR_PRIORITY_SHIFT 0
+#define INT_INFO_ATTR_SEC_STATE_SHIFT 8
+#define INT_INFO_ATTR_CONFIG_SHIFT 9
+#define INT_INFO_ATTR_TYPE_SHIFT 10
+
struct interrupt_info {
uint32_t id;
uint32_t attributes;
diff --git a/inc/hf/interrupt_desc.h b/inc/hf/interrupt_desc.h
index b97bfb0..03d32fc 100644
--- a/inc/hf/interrupt_desc.h
+++ b/inc/hf/interrupt_desc.h
@@ -52,12 +52,6 @@
}
/**
- * Legal values to change the security state of an interrupt.
- */
-#define INT_SEC_STATE_NS 0
-#define INT_SEC_STATE_S 1
-
-/**
* Legal values to enable or disable an interrupt through the
* `INT_RECONFIGURE_ENABLE` command using the `HF_INTERRUPT_RECONFIGURE`
* paravirtualized interface.
@@ -66,16 +60,6 @@
#define INT_ENABLE 1
/**
- * Attributes encoding in the manifest:
-
- * Field Bit(s)
- * ---------------------------
- * Priority 7:0
- * Security_State 8
- * Config(Edge/Level) 9
- * Type(SPI/PPI/SGI) 11:10
- * Reserved 31:12
- *
* Implementation defined Encodings for various fields:
*
* Security_State:
@@ -91,140 +75,23 @@
* - SGI: 0b00
*
*/
+#define INT_DESC_SEC_STATE_NS 0
+#define INT_DESC_SEC_STATE_S 1
#define INT_DESC_TYPE_SPI 2
#define INT_DESC_TYPE_PPI 1
#define INT_DESC_TYPE_SGI 0
-/** Interrupt Descriptor field masks and shifts. */
-
-#define INT_DESC_PRIORITY_SHIFT 0
-#define INT_DESC_SEC_STATE_SHIFT 8
-#define INT_DESC_CONFIG_SHIFT 9
-#define INT_DESC_TYPE_SHIFT 10
-
struct interrupt_descriptor {
uint32_t interrupt_id;
- /**
- * Bit fields Position
- * ---------------------
- * reserved: 7:4
- * type: 3:2
- * config: 1
- * sec_state: 0
- */
- uint8_t type_config_sec_state;
+ uint8_t res : 4;
+ uint8_t type : 2;
+ uint8_t config : 1;
+ uint8_t sec_state : 1;
uint8_t priority;
bool valid;
bool mpidr_valid;
uint64_t mpidr;
bool enabled;
};
-
-/**
- * Helper APIs for accessing interrupt_descriptor member variables.
- */
-static inline uint32_t interrupt_desc_get_id(
- struct interrupt_descriptor int_desc)
-{
- return int_desc.interrupt_id;
-}
-
-static inline uint8_t interrupt_desc_get_sec_state(
- struct interrupt_descriptor int_desc)
-{
- return ((int_desc.type_config_sec_state >> 0) & 1U);
-}
-
-static inline uint8_t interrupt_desc_get_config(
- struct interrupt_descriptor int_desc)
-{
- return ((int_desc.type_config_sec_state >> 1) & 1U);
-}
-
-static inline uint8_t interrupt_desc_get_type(
- struct interrupt_descriptor int_desc)
-{
- return ((int_desc.type_config_sec_state >> 2) & 3U);
-}
-
-static inline uint8_t interrupt_desc_get_priority(
- struct interrupt_descriptor int_desc)
-{
- return int_desc.priority;
-}
-
-static inline uint64_t interrupt_desc_get_mpidr(
- struct interrupt_descriptor int_desc)
-{
- return int_desc.mpidr;
-}
-
-static inline bool interrupt_desc_get_mpidr_valid(
- struct interrupt_descriptor int_desc)
-{
- return int_desc.mpidr_valid;
-}
-
-static inline bool interrupt_desc_get_valid(
- struct interrupt_descriptor int_desc)
-{
- return int_desc.valid;
-}
-
-static inline void interrupt_desc_set_id(struct interrupt_descriptor *int_desc,
- uint32_t interrupt_id)
-{
- int_desc->interrupt_id = interrupt_id;
-}
-
-static inline void interrupt_desc_set_mpidr(
- struct interrupt_descriptor *int_desc, uint64_t mpidr)
-{
- int_desc->mpidr_valid = true;
- int_desc->mpidr = mpidr;
-}
-
-static inline void interrupt_desc_set_mpidr_invalid(
- struct interrupt_descriptor *int_desc)
-{
- int_desc->mpidr_valid = false;
- int_desc->mpidr = 0;
-}
-
-static inline void interrupt_desc_set_type_config_sec_state(
- struct interrupt_descriptor *int_desc, uint8_t value)
-{
- int_desc->type_config_sec_state = value;
-}
-
-static inline void interrupt_desc_set_sec_state(
- struct interrupt_descriptor *int_desc, uint8_t value)
-{
- /*
- * Note that the type_config_sec_state field is 8 bit wide. Modify only
- * the bit[0] of the type_config_sec_state field as it represents the
- * security state of the interrupt.
- */
- int_desc->type_config_sec_state =
- (int_desc->type_config_sec_state & 0xFE) | (value & 0x1);
-}
-
-static inline void interrupt_desc_set_priority(
- struct interrupt_descriptor *int_desc, uint8_t priority)
-{
- int_desc->priority = priority;
-}
-
-static inline void interrupt_desc_set_valid(
- struct interrupt_descriptor *int_desc, bool valid)
-{
- int_desc->valid = valid;
-}
-
-static inline void interrupt_desc_set_enabled(
- struct interrupt_descriptor *int_desc, bool enable)
-{
- int_desc->enabled = enable;
-}
diff --git a/src/arch/aarch64/plat/ffa/spmc.c b/src/arch/aarch64/plat/ffa/spmc.c
index b81fc64..807d5a5 100644
--- a/src/arch/aarch64/plat/ffa/spmc.c
+++ b/src/arch/aarch64/plat/ffa/spmc.c
@@ -2009,19 +2009,18 @@
void plat_ffa_sri_init(struct cpu *cpu)
{
- struct interrupt_descriptor sri_desc = {0};
+ /* Configure as Non Secure SGI. */
+ struct interrupt_descriptor sri_desc = {
+ .interrupt_id = HF_SCHEDULE_RECEIVER_INTID,
+ .type = INT_DESC_TYPE_SGI,
+ .sec_state = INT_DESC_SEC_STATE_NS,
+ .priority = SRI_PRIORITY,
+ .valid = true,
+ };
/* TODO: when supported, make the interrupt driver use cpu structure. */
(void)cpu;
- interrupt_desc_set_id(&sri_desc, HF_SCHEDULE_RECEIVER_INTID);
- interrupt_desc_set_priority(&sri_desc, SRI_PRIORITY);
- interrupt_desc_set_valid(&sri_desc, true);
-
- /* Configure Interrupt as Non-Secure. */
- interrupt_desc_set_type_config_sec_state(&sri_desc,
- INT_DESC_TYPE_SGI << 2);
-
plat_interrupts_configure_interrupt(sri_desc);
}
@@ -2629,7 +2628,7 @@
int_desc = vm_locked.vm->interrupt_desc[k];
/* Interrupt descriptors are populated contiguously. */
- if (!interrupt_desc_get_valid(int_desc)) {
+ if (!int_desc.valid) {
break;
}
vcpu_virt_interrupt_set_enabled(interrupts,
@@ -2859,7 +2858,8 @@
break;
case INT_RECONFIGURE_SEC_STATE:
/* Specify the new security state of the interrupt. */
- if (value != INT_SEC_STATE_NS && value != INT_SEC_STATE_S) {
+ if (value != INT_DESC_SEC_STATE_NS &&
+ value != INT_DESC_SEC_STATE_S) {
dlog_verbose(
"Illegal value %x specified while "
"reconfiguring interrupt %x\n",
diff --git a/src/arch/aarch64/plat/interrupts/gicv3.c b/src/arch/aarch64/plat/interrupts/gicv3.c
index 22f999e..1b5dafb 100644
--- a/src/arch/aarch64/plat/interrupts/gicv3.c
+++ b/src/arch/aarch64/plat/interrupts/gicv3.c
@@ -724,23 +724,22 @@
{
uint32_t core_idx = arch_find_core_pos();
uint32_t config = GIC_INTR_CFG_LEVEL;
- uint32_t intr_num = interrupt_desc_get_id(int_desc);
+ uint32_t intr_num = int_desc.interrupt_id;
CHECK(core_idx < MAX_CPUS);
CHECK(IS_SGI_PPI(intr_num) || IS_SPI(intr_num));
/* Configure the interrupt as either G1S or G1NS. */
- if (interrupt_desc_get_sec_state(int_desc) != 0) {
+ if (int_desc.sec_state != 0) {
gicv3_set_interrupt_type(intr_num, core_idx, INTR_GROUP1S);
} else {
gicv3_set_interrupt_type(intr_num, core_idx, INTR_GROUP1NS);
}
/* Program the interrupt priority. */
- gicv3_set_interrupt_priority(intr_num, core_idx,
- interrupt_desc_get_priority(int_desc));
+ gicv3_set_interrupt_priority(intr_num, core_idx, int_desc.priority);
- if (interrupt_desc_get_config(int_desc) == 0) {
+ if (int_desc.config == 0) {
/* Interrupt is edge-triggered. */
config = GIC_INTR_CFG_EDGE;
}
@@ -763,10 +762,9 @@
if (IS_SPI(intr_num)) {
uint64_t gic_affinity_val;
- if (interrupt_desc_get_mpidr_valid(int_desc)) {
+ if (int_desc.mpidr_valid) {
gic_affinity_val = gicd_irouter_val_from_mpidr(
- interrupt_desc_get_mpidr(int_desc),
- GICV3_IRM_PE);
+ int_desc.mpidr, GICV3_IRM_PE);
} else {
gic_affinity_val = gicd_irouter_val_from_mpidr(
read_msr(MPIDR_EL1), 0U);
@@ -794,8 +792,7 @@
{
assert(int_desc.valid);
- gicv3_disable_interrupt(interrupt_desc_get_id(int_desc),
- arch_find_core_pos());
+ gicv3_disable_interrupt(int_desc.interrupt_id, arch_find_core_pos());
/*
* Interrupt already disabled above. Proceed to (re)configure the
diff --git a/src/load.c b/src/load.c
index 83615f3..2735642 100644
--- a/src/load.c
+++ b/src/load.c
@@ -133,25 +133,23 @@
{
uint32_t attr = interrupt.attributes;
- interrupt_desc_set_id(int_desc, interrupt.id);
- interrupt_desc_set_priority(int_desc,
- (attr >> INT_DESC_PRIORITY_SHIFT) & 0xff);
+ int_desc->interrupt_id = interrupt.id;
+ int_desc->priority = (attr >> INT_INFO_ATTR_PRIORITY_SHIFT) & 0xff;
- /* Refer to the comments in interrupt_descriptor struct definition. */
- interrupt_desc_set_type_config_sec_state(
- int_desc,
- (((attr >> INT_DESC_TYPE_SHIFT) & 0x3) << 2) |
- (((attr >> INT_DESC_CONFIG_SHIFT) & 0x1) << 1) |
- ((attr >> INT_DESC_SEC_STATE_SHIFT) & 0x1));
+ int_desc->type = (attr >> INT_INFO_ATTR_TYPE_SHIFT) & 0x3;
+ int_desc->config = (attr >> INT_INFO_ATTR_CONFIG_SHIFT) & 0x1;
+ int_desc->sec_state = (attr >> INT_INFO_ATTR_SEC_STATE_SHIFT) & 0x1;
if (interrupt.mpidr_valid) {
- interrupt_desc_set_mpidr(int_desc, interrupt.mpidr);
+ int_desc->mpidr_valid = true;
+ int_desc->mpidr = interrupt.mpidr;
} else {
- interrupt_desc_set_mpidr_invalid(int_desc);
+ int_desc->mpidr_valid = false;
+ int_desc->mpidr = 0;
}
- interrupt_desc_set_valid(int_desc, true);
- interrupt_desc_set_enabled(int_desc, true);
+ int_desc->valid = true;
+ int_desc->enabled = true;
}
/**
diff --git a/src/vm.c b/src/vm.c
index a850a65..39af227 100644
--- a/src/vm.c
+++ b/src/vm.c
@@ -1026,7 +1026,8 @@
int_desc = vm_find_interrupt_descriptor(vm_locked, id);
if (int_desc != NULL) {
- interrupt_desc_set_mpidr(int_desc, target_mpidr);
+ int_desc->mpidr_valid = true;
+ int_desc->mpidr = target_mpidr;
}
return int_desc;
@@ -1044,7 +1045,7 @@
int_desc = vm_find_interrupt_descriptor(vm_locked, id);
if (int_desc != NULL) {
- interrupt_desc_set_sec_state(int_desc, sec_state);
+ int_desc->sec_state = sec_state;
}
return int_desc;
@@ -1061,7 +1062,7 @@
int_desc = vm_find_interrupt_descriptor(vm_locked, id);
if (int_desc != NULL) {
- interrupt_desc_set_enabled(int_desc, enable);
+ int_desc->enabled = enable;
}
return int_desc;