Re: [RFC PATCH v4 22/36] i386/tdx: Track RAM entries for TDX VM

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 5/26/2022 3:33 PM, Xiaoyao Li wrote:
> On 5/24/2022 3:37 PM, Gerd Hoffmann wrote:

>>> +        if (e->address == address && e->length == length) {
>>> +            e->type = TDX_RAM_ADDED;
>>> +        } else if (e->address == address) {
>>> +            e->address += length;
>>> +            e->length -= length;
>>> +            tdx_add_ram_entry(address, length, TDX_RAM_ADDED);
>>> +        } else if (e->address + e->length == address + length) {
>>> +            e->length -= length;
>>> +            tdx_add_ram_entry(address, length, TDX_RAM_ADDED);
>>> +        } else {
>>> +            TdxRamEntry tmp = {
>>> +                .address = e->address,
>>> +                .length = e->length,
>>> +            };
>>> +            e->length = address - tmp.address;
>>> +
>>> +            tdx_add_ram_entry(address, length, TDX_RAM_ADDED);
>>> +            tdx_add_ram_entry(address + length,
>>> +                              tmp.address + tmp.length - (address + 
>>> length),
>>> +                              TDX_RAM_UNACCEPTED);
>>> +        }
>>
>> I think all this can be simplified, by
>>    (1) Change the existing entry to cover the accepted ram range.
>>    (2) If there is room before the accepted ram range add a
>>        TDX_RAM_UNACCEPTED entry for that.
>>    (3) If there is room after the accepted ram range add a
>>        TDX_RAM_UNACCEPTED entry for that.
> 
> I implement as below. Please help review.
> 
> +static int tdx_accept_ram_range(uint64_t address, uint64_t length)
> +{
> +    uint64_t head_start, tail_start, head_length, tail_length;
> +    uint64_t tmp_address, tmp_length;
> +    TdxRamEntry *e;
> +    int i;
> +
> +    for (i = 0; i < tdx_guest->nr_ram_entries; i++) {
> +        e = &tdx_guest->ram_entries[i];
> +
> +        if (address + length < e->address ||
> +            e->address + e->length < address) {
> +                continue;
> +        }
> +
> +        /*
> +         * The to-be-accepted ram range must be fully contained by one
> +         * RAM entries
> +         */
> +        if (e->address > address ||
> +            e->address + e->length < address + length) {
> +            return -EINVAL;
> +        }
> +
> +        if (e->type == TDX_RAM_ADDED) {
> +            return -EINVAL;
> +        }
> +
> +        tmp_address = e->address;
> +        tmp_length = e->length;
> +
> +        e->address = address;
> +        e->length = length;
> +        e->type = TDX_RAM_ADDED;
> +
> +        head_length = address - tmp_address;
> +        if (head_length > 0) {
> +            head_start = e->address;
> +            tdx_add_ram_entry(head_start, head_length, 
> TDX_RAM_UNACCEPTED);
> +        }
> +
> +        tail_start = address + length;
> +        if (tail_start < tmp_address + tmp_length) {
> +            tail_length = e->address + e->length - tail_start;
> +            tdx_add_ram_entry(tail_start, tail_length, 
> TDX_RAM_UNACCEPTED);
> +        }
> +
> +        return 0;
> +    }
> +
> +    return -1;
> +}

above is incorrect. I implement fixed one:

+static int tdx_accept_ram_range(uint64_t address, uint64_t length)
+{
+    uint64_t head_start, tail_start, head_length, tail_length;
+    uint64_t tmp_address, tmp_length;
+    TdxRamEntry *e;
+    int i;
+
+    for (i = 0; i < tdx_guest->nr_ram_entries; i++) {
+        e = &tdx_guest->ram_entries[i];
+
+        if (address + length < e->address ||
+            e->address + e->length < address) {
+                continue;
+        }
+
+        /*
+         * The to-be-accepted ram range must be fully contained by one
+         * RAM entries
+         */
+        if (e->address > address ||
+            e->address + e->length < address + length) {
+            return -EINVAL;
+        }
+
+        if (e->type == TDX_RAM_ADDED) {
+            return -EINVAL;
+        }
+
+        tmp_address = e->address;
+        tmp_length = e->length;
+
+        e->address = address;
+        e->length = length;
+        e->type = TDX_RAM_ADDED;
+
+        head_length = address - tmp_address;
+        if (head_length > 0) {
+            head_start = tmp_address;
+            tdx_add_ram_entry(head_start, head_length, TDX_RAM_UNACCEPTED);
+        }
+
+        tail_start = address + length;
+        if (tail_start < tmp_address + tmp_length) {
+            tail_length = tmp_address + tmp_length - tail_start;
+            tdx_add_ram_entry(tail_start, tail_length, TDX_RAM_UNACCEPTED);
+        }
+
+        return 0;
+    }
+
+    return -1;
+}


> 
> 
>> take care,
>>    Gerd
>>
>