Brad Bishop | bec4ebc | 2022-08-03 09:55:16 -0400 | [diff] [blame] | 1 | From 64d5628c8439e4649e9c1da9b9e02ebd5c7fb8cf Mon Sep 17 00:00:00 2001 |
| 2 | From: Ben Horgan <ben.horgan@arm.com> |
| 3 | Date: Thu, 28 Apr 2022 15:53:31 +0000 |
| 4 | Subject: [PATCH 5/5] Revert "fix(ff-a): check receiver's attributes on memory |
| 5 | retrieve" |
| 6 | |
| 7 | This reverts commit a98603aa965e3ff3ca5383249213e2fd1a96d850. |
| 8 | |
| 9 | Change-Id: Ia71ce3ac52e9b2e85578372c24eb8d593b62435f |
| 10 | Signed-off-by: Ben Horgan <ben.horgan@arm.com> |
| 11 | --- |
| 12 | src/ffa_memory.c | 76 ++++++----------- |
| 13 | test/vmapi/el0_partitions/memory_sharing.c | 81 ------------------- |
| 14 | .../primary_with_secondaries/memory_sharing.c | 81 ------------------- |
| 15 | 3 files changed, 25 insertions(+), 213 deletions(-) |
| 16 | |
| 17 | diff --git a/src/ffa_memory.c b/src/ffa_memory.c |
| 18 | index ab47929..2fcc386 100644 |
| 19 | --- a/src/ffa_memory.c |
| 20 | +++ b/src/ffa_memory.c |
| 21 | @@ -1344,42 +1344,6 @@ static struct ffa_value ffa_memory_send_complete( |
| 22 | return ffa_mem_success(share_state->memory_region->handle); |
| 23 | } |
| 24 | |
| 25 | -/** |
| 26 | - * Check that the memory attributes match Hafnium expectations: |
| 27 | - * Normal Memory, Inner shareable, Write-Back Read-Allocate |
| 28 | - * Write-Allocate Cacheable. |
| 29 | - */ |
| 30 | -static struct ffa_value ffa_memory_attributes_validate( |
| 31 | - ffa_memory_access_permissions_t attributes) |
| 32 | -{ |
| 33 | - enum ffa_memory_type memory_type; |
| 34 | - enum ffa_memory_cacheability cacheability; |
| 35 | - enum ffa_memory_shareability shareability; |
| 36 | - |
| 37 | - memory_type = ffa_get_memory_type_attr(attributes); |
| 38 | - if (memory_type != FFA_MEMORY_NORMAL_MEM) { |
| 39 | - dlog_verbose("Invalid memory type %#x, expected %#x.\n", |
| 40 | - memory_type, FFA_MEMORY_NORMAL_MEM); |
| 41 | - return ffa_error(FFA_DENIED); |
| 42 | - } |
| 43 | - |
| 44 | - cacheability = ffa_get_memory_cacheability_attr(attributes); |
| 45 | - if (cacheability != FFA_MEMORY_CACHE_WRITE_BACK) { |
| 46 | - dlog_verbose("Invalid cacheability %#x, expected %#x.\n", |
| 47 | - cacheability, FFA_MEMORY_CACHE_WRITE_BACK); |
| 48 | - return ffa_error(FFA_DENIED); |
| 49 | - } |
| 50 | - |
| 51 | - shareability = ffa_get_memory_shareability_attr(attributes); |
| 52 | - if (shareability != FFA_MEMORY_INNER_SHAREABLE) { |
| 53 | - dlog_verbose("Invalid shareability %#x, expected #%x.\n", |
| 54 | - shareability, FFA_MEMORY_INNER_SHAREABLE); |
| 55 | - return ffa_error(FFA_DENIED); |
| 56 | - } |
| 57 | - |
| 58 | - return (struct ffa_value){.func = FFA_SUCCESS_32}; |
| 59 | -} |
| 60 | - |
| 61 | /** |
| 62 | * Check that the given `memory_region` represents a valid memory send request |
| 63 | * of the given `share_func` type, return the clear flag and permissions via the |
| 64 | @@ -1400,7 +1364,10 @@ static struct ffa_value ffa_memory_send_validate( |
| 65 | uint32_t constituents_length; |
| 66 | enum ffa_data_access data_access; |
| 67 | enum ffa_instruction_access instruction_access; |
| 68 | - struct ffa_value ret; |
| 69 | + ffa_memory_access_permissions_t attributes; |
| 70 | + enum ffa_memory_type memory_type; |
| 71 | + enum ffa_memory_cacheability memory_cacheability; |
| 72 | + enum ffa_memory_shareability memory_shareability; |
| 73 | |
| 74 | assert(permissions != NULL); |
| 75 | |
| 76 | @@ -1536,9 +1503,26 @@ static struct ffa_value ffa_memory_send_validate( |
| 77 | * Normal Memory, Inner shareable, Write-Back Read-Allocate |
| 78 | * Write-Allocate Cacheable. |
| 79 | */ |
| 80 | - ret = ffa_memory_attributes_validate(memory_region->attributes); |
| 81 | - if (ret.func != FFA_SUCCESS_32) { |
| 82 | - return ret; |
| 83 | + attributes = memory_region->attributes; |
| 84 | + memory_type = ffa_get_memory_type_attr(attributes); |
| 85 | + if (memory_type != FFA_MEMORY_NORMAL_MEM) { |
| 86 | + dlog_verbose("Invalid memory type %#x, expected %#x.\n", |
| 87 | + memory_type, FFA_MEMORY_NORMAL_MEM); |
| 88 | + return ffa_error(FFA_INVALID_PARAMETERS); |
| 89 | + } |
| 90 | + |
| 91 | + memory_cacheability = ffa_get_memory_cacheability_attr(attributes); |
| 92 | + if (memory_cacheability != FFA_MEMORY_CACHE_WRITE_BACK) { |
| 93 | + dlog_verbose("Invalid cacheability %#x, expected %#x.\n", |
| 94 | + memory_cacheability, FFA_MEMORY_CACHE_WRITE_BACK); |
| 95 | + return ffa_error(FFA_INVALID_PARAMETERS); |
| 96 | + } |
| 97 | + |
| 98 | + memory_shareability = ffa_get_memory_shareability_attr(attributes); |
| 99 | + if (memory_shareability != FFA_MEMORY_INNER_SHAREABLE) { |
| 100 | + dlog_verbose("Invalid shareability %#x, expected %#x.\n", |
| 101 | + memory_shareability, FFA_MEMORY_INNER_SHAREABLE); |
| 102 | + return ffa_error(FFA_INVALID_PARAMETERS); |
| 103 | } |
| 104 | |
| 105 | return (struct ffa_value){.func = FFA_SUCCESS_32}; |
| 106 | @@ -2376,6 +2360,7 @@ struct ffa_value ffa_memory_retrieve(struct vm_locked to_locked, |
| 107 | * Check permissions from sender against permissions requested by |
| 108 | * receiver. |
| 109 | */ |
| 110 | + /* TODO: Check attributes too. */ |
| 111 | sent_permissions = |
| 112 | memory_region->receivers[0].receiver_permissions.permissions; |
| 113 | sent_data_access = ffa_get_data_access_attr(sent_permissions); |
| 114 | @@ -2453,17 +2438,6 @@ struct ffa_value ffa_memory_retrieve(struct vm_locked to_locked, |
| 115 | panic("Got unexpected FFA_INSTRUCTION_ACCESS_RESERVED. Should " |
| 116 | "be checked before this point."); |
| 117 | } |
| 118 | - |
| 119 | - /* |
| 120 | - * Ensure receiver's attributes are compatible with how Hafnium maps |
| 121 | - * memory: Normal Memory, Inner shareable, Write-Back Read-Allocate |
| 122 | - * Write-Allocate Cacheable. |
| 123 | - */ |
| 124 | - ret = ffa_memory_attributes_validate(retrieve_request->attributes); |
| 125 | - if (ret.func != FFA_SUCCESS_32) { |
| 126 | - goto out; |
| 127 | - } |
| 128 | - |
| 129 | memory_to_attributes = ffa_memory_permissions_to_mode( |
| 130 | permissions, share_state->sender_orig_mode); |
| 131 | ret = ffa_retrieve_check_update( |
| 132 | diff --git a/test/vmapi/el0_partitions/memory_sharing.c b/test/vmapi/el0_partitions/memory_sharing.c |
| 133 | index 3756d7d..c29f029 100644 |
| 134 | --- a/test/vmapi/el0_partitions/memory_sharing.c |
| 135 | +++ b/test/vmapi/el0_partitions/memory_sharing.c |
| 136 | @@ -2160,87 +2160,6 @@ TEST(memory_sharing, ffa_validate_retrieve_req_mbz) |
| 137 | } |
| 138 | } |
| 139 | |
| 140 | -/** |
| 141 | - * Memory can't be shared with arbitrary attributes because Hafnium maps pages |
| 142 | - * with hardcoded values and doesn't support custom mappings. |
| 143 | - */ |
| 144 | -TEST(memory_sharing, ffa_validate_retrieve_req_attributes) |
| 145 | -{ |
| 146 | - struct ffa_value ret; |
| 147 | - struct mailbox_buffers mb = set_up_mailbox(); |
| 148 | - uint32_t msg_size; |
| 149 | - ffa_memory_handle_t handle; |
| 150 | - |
| 151 | - struct ffa_value (*send_function[])(uint32_t, uint32_t) = { |
| 152 | - ffa_mem_share, |
| 153 | - ffa_mem_lend, |
| 154 | - }; |
| 155 | - |
| 156 | - struct ffa_memory_region_constituent constituents[] = { |
| 157 | - {.address = (uint64_t)pages, .page_count = 2}, |
| 158 | - {.address = (uint64_t)pages + PAGE_SIZE * 3, .page_count = 1}, |
| 159 | - }; |
| 160 | - |
| 161 | - SERVICE_SELECT(SERVICE_VM1, "ffa_memory_share_fail", mb.send); |
| 162 | - |
| 163 | - struct { |
| 164 | - enum ffa_memory_type memory_type; |
| 165 | - enum ffa_memory_cacheability memory_cacheability; |
| 166 | - enum ffa_memory_shareability memory_shareability; |
| 167 | - } invalid_attributes[] = { |
| 168 | - /* Invalid memory type */ |
| 169 | - {FFA_MEMORY_DEVICE_MEM, FFA_MEMORY_CACHE_WRITE_BACK, |
| 170 | - FFA_MEMORY_INNER_SHAREABLE}, |
| 171 | - /* Invalid cacheability */ |
| 172 | - {FFA_MEMORY_NORMAL_MEM, FFA_MEMORY_CACHE_NON_CACHEABLE, |
| 173 | - FFA_MEMORY_INNER_SHAREABLE}, |
| 174 | - /* Invalid shareability */ |
| 175 | - {FFA_MEMORY_NORMAL_MEM, FFA_MEMORY_CACHE_WRITE_BACK, |
| 176 | - FFA_MEMORY_SHARE_NON_SHAREABLE}, |
| 177 | - {FFA_MEMORY_NORMAL_MEM, FFA_MEMORY_CACHE_WRITE_BACK, |
| 178 | - FFA_MEMORY_OUTER_SHAREABLE}}; |
| 179 | - |
| 180 | - for (uint32_t i = 0; i < ARRAY_SIZE(send_function); i++) { |
| 181 | - /* Prepare memory region, and set all flags */ |
| 182 | - EXPECT_EQ(ffa_memory_region_init( |
| 183 | - mb.send, HF_MAILBOX_SIZE, HF_PRIMARY_VM_ID, |
| 184 | - SERVICE_VM1, constituents, |
| 185 | - ARRAY_SIZE(constituents), 0, 0, |
| 186 | - FFA_DATA_ACCESS_RW, |
| 187 | - FFA_INSTRUCTION_ACCESS_NOT_SPECIFIED, |
| 188 | - FFA_MEMORY_NORMAL_MEM, |
| 189 | - FFA_MEMORY_CACHE_WRITE_BACK, |
| 190 | - FFA_MEMORY_INNER_SHAREABLE, NULL, &msg_size), |
| 191 | - 0); |
| 192 | - |
| 193 | - ret = send_function[i](msg_size, msg_size); |
| 194 | - EXPECT_EQ(ret.func, FFA_SUCCESS_32); |
| 195 | - |
| 196 | - handle = ffa_mem_success_handle(ret); |
| 197 | - |
| 198 | - for (uint32_t j = 0; j < ARRAY_SIZE(invalid_attributes); ++j) { |
| 199 | - msg_size = ffa_memory_retrieve_request_init( |
| 200 | - mb.send, handle, HF_PRIMARY_VM_ID, SERVICE_VM1, |
| 201 | - 0, 0, FFA_DATA_ACCESS_RW, |
| 202 | - FFA_INSTRUCTION_ACCESS_NOT_SPECIFIED, |
| 203 | - invalid_attributes[j].memory_type, |
| 204 | - invalid_attributes[j].memory_cacheability, |
| 205 | - invalid_attributes[j].memory_shareability); |
| 206 | - |
| 207 | - EXPECT_LE(msg_size, HF_MAILBOX_SIZE); |
| 208 | - |
| 209 | - EXPECT_EQ(ffa_msg_send(HF_PRIMARY_VM_ID, SERVICE_VM1, |
| 210 | - msg_size, 0) |
| 211 | - .func, |
| 212 | - FFA_SUCCESS_32); |
| 213 | - |
| 214 | - ffa_run(SERVICE_VM1, 0); |
| 215 | - } |
| 216 | - |
| 217 | - EXPECT_EQ(ffa_mem_reclaim(handle, 0).func, FFA_SUCCESS_32); |
| 218 | - } |
| 219 | -} |
| 220 | - |
| 221 | /** |
| 222 | * If memory is shared can't request zeroing of memory at both send and |
| 223 | * relinquish. |
| 224 | diff --git a/test/vmapi/primary_with_secondaries/memory_sharing.c b/test/vmapi/primary_with_secondaries/memory_sharing.c |
| 225 | index 6080709..4bcf252 100644 |
| 226 | --- a/test/vmapi/primary_with_secondaries/memory_sharing.c |
| 227 | +++ b/test/vmapi/primary_with_secondaries/memory_sharing.c |
| 228 | @@ -2307,87 +2307,6 @@ TEST(memory_sharing, ffa_validate_retrieve_req_mbz) |
| 229 | } |
| 230 | } |
| 231 | |
| 232 | -/** |
| 233 | - * Memory can't be shared with arbitrary attributes because Hafnium maps pages |
| 234 | - * with hardcoded values and doesn't support custom mappings. |
| 235 | - */ |
| 236 | -TEST(memory_sharing, ffa_validate_retrieve_req_attributes) |
| 237 | -{ |
| 238 | - struct ffa_value ret; |
| 239 | - struct mailbox_buffers mb = set_up_mailbox(); |
| 240 | - uint32_t msg_size; |
| 241 | - ffa_memory_handle_t handle; |
| 242 | - |
| 243 | - struct ffa_value (*send_function[])(uint32_t, uint32_t) = { |
| 244 | - ffa_mem_share, |
| 245 | - ffa_mem_lend, |
| 246 | - }; |
| 247 | - |
| 248 | - struct ffa_memory_region_constituent constituents[] = { |
| 249 | - {.address = (uint64_t)pages, .page_count = 2}, |
| 250 | - {.address = (uint64_t)pages + PAGE_SIZE * 3, .page_count = 1}, |
| 251 | - }; |
| 252 | - |
| 253 | - SERVICE_SELECT(SERVICE_VM1, "ffa_memory_share_fail_denied", mb.send); |
| 254 | - |
| 255 | - struct { |
| 256 | - enum ffa_memory_type memory_type; |
| 257 | - enum ffa_memory_cacheability memory_cacheability; |
| 258 | - enum ffa_memory_shareability memory_shareability; |
| 259 | - } invalid_attributes[] = { |
| 260 | - /* Invalid memory type */ |
| 261 | - {FFA_MEMORY_DEVICE_MEM, FFA_MEMORY_CACHE_WRITE_BACK, |
| 262 | - FFA_MEMORY_INNER_SHAREABLE}, |
| 263 | - /* Invalid cacheability */ |
| 264 | - {FFA_MEMORY_NORMAL_MEM, FFA_MEMORY_CACHE_NON_CACHEABLE, |
| 265 | - FFA_MEMORY_INNER_SHAREABLE}, |
| 266 | - /* Invalid shareability */ |
| 267 | - {FFA_MEMORY_NORMAL_MEM, FFA_MEMORY_CACHE_WRITE_BACK, |
| 268 | - FFA_MEMORY_SHARE_NON_SHAREABLE}, |
| 269 | - {FFA_MEMORY_NORMAL_MEM, FFA_MEMORY_CACHE_WRITE_BACK, |
| 270 | - FFA_MEMORY_OUTER_SHAREABLE}}; |
| 271 | - |
| 272 | - for (uint32_t i = 0; i < ARRAY_SIZE(send_function); i++) { |
| 273 | - /* Prepare memory region, and set all flags */ |
| 274 | - EXPECT_EQ(ffa_memory_region_init( |
| 275 | - mb.send, HF_MAILBOX_SIZE, HF_PRIMARY_VM_ID, |
| 276 | - SERVICE_VM1, constituents, |
| 277 | - ARRAY_SIZE(constituents), 0, 0, |
| 278 | - FFA_DATA_ACCESS_RW, |
| 279 | - FFA_INSTRUCTION_ACCESS_NOT_SPECIFIED, |
| 280 | - FFA_MEMORY_NORMAL_MEM, |
| 281 | - FFA_MEMORY_CACHE_WRITE_BACK, |
| 282 | - FFA_MEMORY_INNER_SHAREABLE, NULL, &msg_size), |
| 283 | - 0); |
| 284 | - |
| 285 | - ret = send_function[i](msg_size, msg_size); |
| 286 | - EXPECT_EQ(ret.func, FFA_SUCCESS_32); |
| 287 | - |
| 288 | - handle = ffa_mem_success_handle(ret); |
| 289 | - |
| 290 | - for (uint32_t j = 0; j < ARRAY_SIZE(invalid_attributes); ++j) { |
| 291 | - msg_size = ffa_memory_retrieve_request_init( |
| 292 | - mb.send, handle, HF_PRIMARY_VM_ID, SERVICE_VM1, |
| 293 | - 0, 0, FFA_DATA_ACCESS_RW, |
| 294 | - FFA_INSTRUCTION_ACCESS_NOT_SPECIFIED, |
| 295 | - invalid_attributes[j].memory_type, |
| 296 | - invalid_attributes[j].memory_cacheability, |
| 297 | - invalid_attributes[j].memory_shareability); |
| 298 | - |
| 299 | - EXPECT_LE(msg_size, HF_MAILBOX_SIZE); |
| 300 | - |
| 301 | - EXPECT_EQ(ffa_msg_send(HF_PRIMARY_VM_ID, SERVICE_VM1, |
| 302 | - msg_size, 0) |
| 303 | - .func, |
| 304 | - FFA_SUCCESS_32); |
| 305 | - |
| 306 | - ffa_run(SERVICE_VM1, 0); |
| 307 | - } |
| 308 | - |
| 309 | - EXPECT_EQ(ffa_mem_reclaim(handle, 0).func, FFA_SUCCESS_32); |
| 310 | - } |
| 311 | -} |
| 312 | - |
| 313 | /** |
| 314 | * If memory is shared can't request zeroing of memory at both send and |
| 315 | * relinquish. |
| 316 | -- |
| 317 | 2.17.1 |
| 318 | |