i#4400: AArch64: use regular memory operands in dcache instructions

[AssadHashmi]

Data cache operations like DC ZVA should handle addresses as regular store instructions so that clients can analyse runs based on actual start address and cache line size.

Issues: #4400, #2626

[AssadHashmi]

This patch continues the work started in https://github.com/DynamoRIO/dynamorio/pull/5235. I think I've addressed all of @derekbruening's bullet points at https://github.com/DynamoRIO/dynamorio/pull/5235/files#r761331940 except the last two:

Adding support for the new flag to drutil_insert_get_mem_addr()

I don't understand what needs to be done in this function because it handles the base address's extend, scale etc. I can't see what needs to be done for alignment.

A bump in the offline trace version with conditional handling of the existing workaround

Does this refer to the changes done by https://github.com/DynamoRIO/dynamorio/pull/4440 ? I can't see back_align_dc_zva_memref_opnd() in the code any more so that PR's changes must be out-of-date by now ? I'm guessing that opnd_set_base_aligned() needs to be added in drcache related code ?

[derekbruening]

Adding support for the new flag to drutil_insert_get_mem_addr()

I don't understand what needs to be done in this function because it handles the base address's extend, scale etc. I can't see what needs to be done for alignment.

A DC ZVA instruction's base-disp operand's base is a register whose value is a potentially unaligned address. drutil_insert_get_mem_addr() needs to back-align it so the actual address range that DC ZVA references is presented to the tracing tool.

A bump in the offline trace version with conditional handling of the existing workaround

Does this refer to the changes done by #4440 ? I can't see back_align_dc_zva_memref_opnd() in the code any more so that PR's changes must be out-of-date by now ? I'm guessing that opnd_set_base_aligned() needs to be added in drcache related code ?

This code https://github.com/DynamoRIO/dynamorio/blob/master/clients/drcachesim/tracer/raw2trace.h#L1486 should only be invoked for old traces; traces with the version bump will have the new drutil_insert_get_mem_addr() which will have already back-aligned the address and inserted the proper size (which is technically not necessarily the cache line size used in that workaround).

[derekbruening]

Another spot that should look at the new opnd flag and do the alignment is opnd_compute_address (away from computer, I think that’s the name).

On Fri, Dec 24, 2021 at 8:09 AM Assad Hashmi @.***> wrote:

@.**** commented on this pull request.

In core/ir/aarch64/codec.c https://github.com/DynamoRIO/dynamorio/pull/5265#discussion_r775048457:

 *opnd = opnd_create_base_disp(decode_reg(extract_uint(enc, 0, 5), true, false),

•                              DR_REG_NULL, 0, 0, OPSZ_sys);
  

•                              DR_REG_NULL, 0, 0, OPSZ_CACHE_LINE);
  

• if (!opnd_set_base_aligned(opnd, true))

My understanding (based on @derekbruening https://github.com/derekbruening's explanation about back-alignment) is that the alignment should be done by drutil_insert_get_mem_addr() which clients will call.

— Reply to this email directly, view it on GitHub https://github.com/DynamoRIO/dynamorio/pull/5265#discussion_r775048457, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRVIMXRPSXENXBVSFEB3I3USSLMRANCNFSM5KTDD3TA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

[AssadHashmi]

run arm tests