On 02.02.21 17:44, Bram Hooimeijer wrote:
> The procedures for shrinking and extending the cat_mask of the rool cell
> affect other, non-root, cells as well, if these cell use the root COS.
> That is, when cells are configured without cache regions. The code is
> updated to reflect these changes not only in the root-cell, but in the
> struct corresponding to these non-root cells as well.
>
> Fixes: 3f04eb1753bb ("x86: Introduce Cache Allocation Technology support
> for Intel CPUs")
>
> Signed-off-by: Bram Hooimeijer <[email protected]>
> ---
> hypervisor/arch/x86/cat.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/hypervisor/arch/x86/cat.c b/hypervisor/arch/x86/cat.c
> index f6719b1e..42fd83d9 100644
> --- a/hypervisor/arch/x86/cat.c
> +++ b/hypervisor/arch/x86/cat.c
> @@ -60,6 +60,13 @@ retry:
> return cos;
> }
>
> +/**
> + * Merge available bits in the CBM back to root by modifying the cat_mask of
> + * the root.
> + *
> + * It is the callers responsibility to call cat_update_cell(&root_cell), and
> + * to modify the cat_mask of the non-root cells sharing the root's COS.
> + */
> static bool merge_freed_mask_to_root(void)
> {
> bool updated = false;
> @@ -86,6 +93,7 @@ static bool shrink_root_cell_mask(u64 cell_mask)
> {
> unsigned int lo_mask_start, lo_mask_len;
> u64 lo_mask;
> + struct cell *cell;
>
> if ((root_cell.arch.cat_mask & ~cell_mask) == 0) {
> /*
> @@ -125,8 +133,17 @@ static bool shrink_root_cell_mask(u64 cell_mask)
> }
> }
>
> - printk("CAT: Shrunk root cell bitmask to %08llx\n",
> - root_cell.arch.cat_mask);
> + /* Cells using the root COS are also affected by shrinking. */
> + printk("CAT: Set COS %d bitmask to %08llx for root cell",
> + root_cell.arch.cos, root_cell.arch.cat_mask);
> + for_each_non_root_cell(cell)
> + if (cell->arch.cos == root_cell.arch.cos) {
> + cell->arch.cat_mask = root_cell.arch.cat_mask;
> + printk(", %s", cell->config->name);
> + }
> + printk("\n");
> + /* However, updating the bitmask once suffices. This can be done
> + * during code execution, no suspense required. (SDM 17.19.6.3) */
> cat_update_cell(&root_cell);
>
> /* Drop this mask from the freed mask in case it was queued there. */
> @@ -201,8 +218,14 @@ static void cat_cell_exit(struct cell *cell)
> freed_mask |= cell->arch.cat_mask & orig_root_mask;
>
> if (merge_freed_mask_to_root()) {
> - printk("CAT: Extended root cell bitmask to %08llx\n",
> - root_cell.arch.cat_mask);
> + printk("CAT: Extended COS %d bitmask to %08llx for root cell",
> + root_cell.arch.cos, root_cell.arch.cat_mask);
> + for_each_non_root_cell(oth_cell)
> + if (oth_cell->arch.cos == root_cell.arch.cos) {
> + oth_cell->arch.cat_mask =
> root_cell.arch.cat_mask;
> + printk(", %s", cell->config->name);
> + }
> + printk("\n");
> cat_update_cell(&root_cell);
> }
> }
>
Valid point that arch.cat_mask for the sharing cell gets out of sync.
But what is the practical impact? We don't run cat_update_cell() for
sharing cells, and cat_cell_exit() does nothing in that case. This is
first of all to understand the impact of the issue.
If there is impact, I'm considering to use (also) a mask pointer so that
there is no need to walk all cells on root cell updates.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/fc2280e8-7800-2a80-a886-32179af203f2%40siemens.com.
