[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
>>> On 17.12.16 at 00:18, <boris.ostrovsky@xxxxxxxxxx> wrote: > +static int acpi_cpumap_access_common(struct domain *d, > + int dir, unsigned int port, > + unsigned int bytes, uint32_t *val) > +{ > + unsigned int first_byte = port - XEN_ACPI_CPU_MAP; > + > + BUILD_BUG_ON(XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN > + >= ACPI_GPE0_BLK_ADDRESS_V1); Just > afaict. > + > + if ( dir == XEN_DOMCTL_ACPI_READ ) > + { > + uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0; > + > + /* > + * Clear bits that we are about to read to in case we > + * copy fewer than @bytes. > + */ > + *val &= mask; > + > + if ( ((d->max_vcpus + 7) / 8) > first_byte ) > + memcpy(val, (uint8_t *)d->avail_vcpus + first_byte, > + min(bytes, ((d->max_vcpus + 7) / 8) - first_byte)); > + } > + else > + /* Guests do not write CPU map */ > + return X86EMUL_UNHANDLEABLE; Perhaps make this an early bail, reducing overall indentation? > +static int acpi_access_common(struct domain *d, > + int dir, unsigned int port, > + unsigned int bytes, uint32_t *val) > +{ > + uint16_t *sts = NULL, *en = NULL; > + const uint16_t *mask_sts = NULL, *mask_en = NULL; > + static const uint16_t pm1a_sts_mask = ACPI_BITMASK_GLOBAL_LOCK_STATUS; > + static const uint16_t pm1a_en_mask = ACPI_BITMASK_GLOBAL_LOCK_ENABLE; > + static const uint16_t gpe0_sts_mask = 1U << XEN_ACPI_GPE0_CPUHP_BIT; > + static const uint16_t gpe0_en_mask = 1U << XEN_ACPI_GPE0_CPUHP_BIT; > + > + ASSERT(!has_acpi_dm_ff(d)); > + > + switch ( port ) > + { > + case ACPI_PM1A_EVT_BLK_ADDRESS_V1 ... > + ACPI_PM1A_EVT_BLK_ADDRESS_V1 + > + sizeof(d->arch.hvm_domain.acpi.pm1a_sts) + > + sizeof(d->arch.hvm_domain.acpi.pm1a_en): > + > + sts = &d->arch.hvm_domain.acpi.pm1a_sts; > + en = &d->arch.hvm_domain.acpi.pm1a_en; > + mask_sts = &pm1a_sts_mask; > + mask_en = &pm1a_en_mask; > + break; > + > + case ACPI_GPE0_BLK_ADDRESS_V1 ... > + ACPI_GPE0_BLK_ADDRESS_V1 + > + sizeof(d->arch.hvm_domain.acpi.gpe0_sts) + > + sizeof(d->arch.hvm_domain.acpi.gpe0_en): > + > + sts = &d->arch.hvm_domain.acpi.gpe0_sts; > + en = &d->arch.hvm_domain.acpi.gpe0_en; > + mask_sts = &gpe0_sts_mask; > + mask_en = &gpe0_en_mask; > + break; > + > + default: > + return X86EMUL_UNHANDLEABLE; > + } > + > + if ( dir == XEN_DOMCTL_ACPI_READ ) > + { > + uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0; > + uint32_t data = (((uint32_t)*en) << 16) | *sts; There's one pair of pointless parentheses around the cast expression here. > + > + data >>= 8 * (port & 3); > + *val = (*val & mask) | (data & ~mask); > + } > + else > + { > + uint32_t v = *val; > + > + /* Status register is write-1-to-clear by guests */ > + switch ( port & 3 ) > + { > + case 0: > + *sts &= ~(v & 0xff); > + *sts &= *mask_sts; I can understand the first &=, but why would a read have this second (side) effect? I could see some sort of need for such only when you were setting any flags. > + if ( !--bytes ) > + break; > + v >>= 8; > + /* fallthrough */ > + case 1: > + *sts &= ~((v & 0xff) << 8); > + *sts &= *mask_sts; > + if ( !--bytes ) > + break; > + v >>= 8; > + /* fallthrough */ > + case 2: > + *en = ((*en & 0xff00) | (v & 0xff)) & *mask_en; > + if ( !--bytes ) > + break; > + v >>= 8; > + /* fallthrough */ > + case 3: > + *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en; > + break; > + } > + } > + > + return X86EMUL_OKAY; > +} > + > + > int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, No double blank lines please. > @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, > static int acpi_guest_access(int dir, unsigned int port, > unsigned int bytes, uint32_t *val) > { > - return X86EMUL_UNHANDLEABLE; > + return acpi_access_common(current->domain, > + (dir == IOREQ_READ) ? > + XEN_DOMCTL_ACPI_READ: XEN_DOMCTL_ACPI_WRITE, > + port, bytes, val); > } I don't think this one is meant to be used by the domctl, so I don't see why you need a helper here. That would also eliminate the seemingly unnecessary use of XEN_DOMCTL_* here (I think you already had said this was an oversight in an earlier reply). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |